From c7a3d3634354bdb804345944dfe5f7c890200871 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Mon, 16 Aug 2021 15:30:04 -0700 Subject: [PATCH 1/2] SHIBUI-2023 Adding regex field to Groups --- .../security/controller/GroupController.java | 7 +- .../GroupControllerExceptionHandler.java | 13 +- .../exception/InvalidGroupRegexException.java | 7 + .../admin/ui/security/model/Group.java | 7 +- .../ui/security/service/GroupServiceImpl.java | 37 ++++- .../ui/security/service/IGroupService.java | 7 +- .../ui/security/service/UserService.java | 8 +- .../security/service/GroupServiceTests.groovy | 148 ++++++++++++++++++ .../security/service/UserServiceTests.groovy | 2 +- 9 files changed, 215 insertions(+), 21 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/InvalidGroupRegexException.java create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceTests.groovy diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java index c0fc0a8ea..bcb1b047c 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.security.controller; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -32,7 +33,7 @@ public class GroupController { @Secured("ROLE_ADMIN") @PostMapping @Transactional - public ResponseEntity create(@RequestBody Group group) throws GroupExistsConflictException { + public ResponseEntity create(@RequestBody Group group) throws GroupExistsConflictException, InvalidGroupRegexException { Group result = groupService.createGroup(group); return ResponseEntity.status(HttpStatus.CREATED).body(result); } @@ -64,8 +65,8 @@ public ResponseEntity getOne(@PathVariable String resourceId) throws EntityNo @Secured("ROLE_ADMIN") @PutMapping @Transactional - public ResponseEntity update(@RequestBody Group group) throws EntityNotFoundException { + public ResponseEntity update(@RequestBody Group group) throws EntityNotFoundException, InvalidGroupRegexException { Group result = groupService.updateGroup(group); return ResponseEntity.ok(result); } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java index e8bb3b4af..39778e21a 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.security.controller; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -43,4 +44,14 @@ public ResponseEntity handleGroupExistsConflict(GroupExistsConflictException return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.METHOD_NOT_ALLOWED.value()), e.getMessage())); } -} + + @ExceptionHandler({ InvalidGroupRegexException.class }) + public ResponseEntity handleInvalidGroupRegexException(InvalidGroupRegexException e, WebRequest request) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.BAD_REQUEST).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.BAD_REQUEST.value()), e.getMessage())); + } + +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/InvalidGroupRegexException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/InvalidGroupRegexException.java new file mode 100644 index 000000000..ae9a034d1 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/InvalidGroupRegexException.java @@ -0,0 +1,7 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.exception; + +public class InvalidGroupRegexException extends Exception { + public InvalidGroupRegexException(String message) { + super(message); + } +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java index 47b607a87..6a986040c 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java @@ -23,6 +23,8 @@ @EntityListeners(GroupUpdatedEntityListener.class) @Entity(name = "user_groups") public class Group implements Owner { + public static final String DEFAULT_REGEX = "/(?:)/"; //non-capturing + @Transient @JsonIgnore public static Group ADMIN_GROUP; @@ -47,7 +49,7 @@ public class Group implements Owner { private String resourceId = UUID.randomUUID().toString(); @Column(name = "validation_regex") - private String validationRegex = "/*"; + private String validationRegex = DEFAULT_REGEX; /** * Define a Group object based on the user @@ -55,7 +57,8 @@ public class Group implements Owner { public Group(User user) { resourceId = user.getUsername(); name = user.getUsername(); - description = "default user-group";new String().matches(""); + description = "default user-group"; + validationRegex = DEFAULT_REGEX; } @Override 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 2501f007e..7f461c816 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,18 +1,20 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service; -import java.util.List; - -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.exception.EntityNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository; import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository; import lombok.NoArgsConstructor; +import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.List; +import java.util.regex.Pattern; @Service @NoArgsConstructor @@ -30,7 +32,7 @@ public GroupServiceImpl(GroupsRepository repo, OwnershipRepository ownershipRepo @Override @Transactional - public Group createGroup(Group group) throws GroupExistsConflictException { + public Group createGroup(Group group) throws GroupExistsConflictException, InvalidGroupRegexException { Group foundGroup = find(group.getResourceId()); // If already defined, we don't want to create a new one, nor do we want this call update the definition if (foundGroup != null) { @@ -38,6 +40,7 @@ public Group createGroup(Group group) throws GroupExistsConflictException { String.format("Call update (PUT) to modify the group with resource id: [%s] and name: [%s]", foundGroup.getResourceId(), foundGroup.getName())); } + validateGroupRegex(group); return groupRepository.save(group); } @@ -61,6 +64,7 @@ public void ensureAdminGroupExists() { g = new Group(); g.setName("ADMIN-GROUP"); g.setResourceId("admingroup"); + g.setValidationRegex("/*"); // Everything g = groupRepository.save(g); } Group.ADMIN_GROUP = g; @@ -78,12 +82,29 @@ public List findAll() { } @Override - public Group updateGroup(Group group) throws EntityNotFoundException { + public Group updateGroup(Group group) throws EntityNotFoundException, InvalidGroupRegexException { Group g = find(group.getResourceId()); if (g == null) { throw new EntityNotFoundException(String.format("Unable to find group with resource id: [%s] and name: [%s]", group.getResourceId(), group.getName())); } + validateGroupRegex(group); return groupRepository.save(group); } + + /** + * If the regex is missing, go with the default "anything goes" regex, otherwise validate that the pattern is valid to use. + */ + private void validateGroupRegex(Group group) throws InvalidGroupRegexException { + if (StringUtils.isEmpty(group.getValidationRegex())) { + group.setValidationRegex(Group.DEFAULT_REGEX); + return; + } + try { + Pattern.compile(group.getValidationRegex()); + } + catch (Exception e) { + throw new InvalidGroupRegexException("Invalid Regular Expression [ " + group.getValidationRegex() + " ]"); + } + } } \ No newline at end of file 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 1c4229d84..95314123f 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 @@ -6,11 +6,12 @@ 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; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; public interface IGroupService { - Group createGroup(Group group) throws GroupExistsConflictException; + Group createGroup(Group group) throws GroupExistsConflictException, InvalidGroupRegexException; void deleteDefinition(String resourceId) throws EntityNotFoundException, GroupDeleteException; @@ -20,6 +21,6 @@ public interface IGroupService { List findAll(); - Group updateGroup(Group g) throws EntityNotFoundException; + Group updateGroup(Group g) throws EntityNotFoundException, InvalidGroupRegexException; -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index a04e2b574..f6ae4dfd8 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -4,6 +4,7 @@ import java.util.Optional; import java.util.Set; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import org.apache.commons.lang.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.context.SecurityContextHolder; @@ -147,7 +148,8 @@ public User save(User user) { try { g = groupService.createGroup(g); } - catch (GroupExistsConflictException e) { + catch (GroupExistsConflictException | InvalidGroupRegexException e) { + // Invalid shouldn't happen for a group created this way. g = groupService.find(user.getUsername()); } } else { @@ -165,8 +167,8 @@ public User save(User user) { Ownership o = ownershipRepository.saveAndFlush(new Ownership(newGroup, user)); g = groupService.createGroup(newGroup); } - catch (GroupExistsConflictException e) { - // we just checked, this shouldn't happen + catch (GroupExistsConflictException | InvalidGroupRegexException e) { + // this shouldn't happen g = ug; } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceTests.groovy new file mode 100644 index 000000000..057cfb576 --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceTests.groovy @@ -0,0 +1,148 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.service + +import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.CustomPropertiesConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration +import edu.internet2.tier.shibboleth.admin.ui.security.controller.GroupController +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group +import edu.internet2.tier.shibboleth.admin.ui.security.model.Role +import edu.internet2.tier.shibboleth.admin.ui.security.model.User +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository +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.boot.test.context.TestConfiguration +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Profile +import org.springframework.data.jpa.repository.config.EnableJpaRepositories +import org.springframework.test.annotation.DirtiesContext +import org.springframework.test.context.ActiveProfiles +import org.springframework.test.context.ContextConfiguration +import org.springframework.test.web.servlet.setup.MockMvcBuilders +import org.springframework.transaction.annotation.Transactional +import org.springframework.web.servlet.LocaleResolver +import org.springframework.web.servlet.i18n.SessionLocaleResolver +import spock.lang.Specification + +@DataJpaTest +@ContextConfiguration(classes=[CoreShibUiConfiguration, InternationalizationConfiguration, SearchConfiguration, LocalConfig, edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration]) +@EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) +@EntityScan("edu.internet2.tier.shibboleth.admin.ui") +@DirtiesContext +@ActiveProfiles(["local"]) +class GroupServiceTests extends Specification { + @Autowired + GroupServiceForTesting groupService + + @Autowired + RoleRepository roleRepository + + @Autowired + UserService userService + + @Transactional + def setup() { + groupService.ensureAdminGroupExists() +// Group g = new Group() +// g.setResourceId("twitter") +// g.setName("twitter") +// // This is valid for a url with "twitter" in it +// g.setValidationRegex("") +// g = groupService.createGroup(g) + + if (roleRepository.count() == 0) { + def roles = [new Role().with { + name = 'ROLE_ADMIN' + it + }, new Role().with { + name = 'ROLE_USER' + it + }, new Role().with { + name = 'ROLE_NONE' + it + }] + roles.each { + roleRepository.save(it) + } + } + + Optional adminRole = roleRepository.findByName("ROLE_ADMIN") + User adminUser = new User(username: "admin", roles: [adminRole.get()], password: "foo") + userService.save(adminUser) + + Optional userRole = roleRepository.findByName("ROLE_USER") + User user = new User(username: "someUser", roles:[userRole.get()], password: "foo") + userService.save(user) + } + + def "Test the validation for regex works"() { + given: + Group g = new Group() + g.setResourceId("twitter") + g.setName("twitter") + g.setValidationRegex(null) + + when: + try { + g = groupService.createGroup(g) + } catch (Exception shouldNotOccur) { + false + } + + then: + g.getValidationRegex() == Group.DEFAULT_REGEX + + when: + g.setValidationRegex("/*") + try { + g = groupService.updateGroup(g) + } catch (Exception shouldNotOccur) { + false + } + + then: + g.getValidationRegex() == "/*" + + when: + g.setValidationRegex("^(http:\\\\/\\\\/www\\\\.|https:\\\\/\\\\/www\\\\.|http:\\\\/\\\\/|https:\\\\/\\\\/)?[a-z0-9]+([\\\\-\\\\.]twitter+)\\\\.[a-z]{2,5}(:[0-9]{1,5})?(\\\\/.*)?\\\$") + try { + g = groupService.updateGroup(g) + } catch (Exception shouldNotOccur) { + false + } + + then: + g.getValidationRegex() == "^(http:\\\\/\\\\/www\\\\.|https:\\\\/\\\\/www\\\\.|http:\\\\/\\\\/|https:\\\\/\\\\/)?[a-z0-9]+([\\\\-\\\\.]twitter+)\\\\.[a-z]{2,5}(:[0-9]{1,5})?(\\\\/.*)?\\\$" + + when: + g.setValidationRegex("*") + + then: + try { + g = groupService.updateGroup(g) + false + } catch (InvalidGroupRegexException shouldOccur) { + true + } + } + + @TestConfiguration + @Profile("local") + static class LocalConfig { + @Bean + GroupServiceForTesting groupServiceForTesting(GroupsRepository repo, OwnershipRepository ownershipRepository) { + GroupServiceForTesting result = new GroupServiceForTesting(new GroupServiceImpl().with { + it.groupRepository = repo + it.ownershipRepository = ownershipRepository + return it + }) + result.ensureAdminGroupExists() + return result + } + } +} \ No newline at end of file diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy index b6431f79f..2ef663fbc 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/UserServiceTests.groovy @@ -258,7 +258,7 @@ class UserServiceTests extends Specification { gbUpdated.ownedItems.size() == 1 } - @org.springframework.boot.test.context.TestConfiguration + @TestConfiguration @Profile("local") static class LocalConfig { @Bean From 86066712f55759b1f42210103e78a657a45aaaa5 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Mon, 16 Aug 2021 15:38:24 -0700 Subject: [PATCH 2/2] SHIBUI-2023 Adding regex field to Groups --- .../MigrationTasksContextLoadedListener.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java index f14d35763..5e6e64b6c 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/auto/MigrationTasksContextLoadedListener.java @@ -1,5 +1,7 @@ package edu.internet2.tier.shibboleth.admin.ui.configuration.auto; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.InvalidGroupRegexException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; @@ -14,6 +16,8 @@ import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; +import java.util.List; + /** * After the context loads, do any needed migration tasks */ @@ -62,6 +66,17 @@ private void doshibui_1740_migration() { userService.save(user); // this will ensure group is set as the default user group } }); - + + // SHIBUI-1743: Adding regex expression to groups + groupService.findAll().forEach(g -> { + g.setValidationRegex(Group.DEFAULT_REGEX); + try { + groupService.updateGroup(g); + } + catch (Exception e) { + // Shouldn't happen + e.printStackTrace(); + } + }); } -} +} \ No newline at end of file