From b625984d70d22f0c7e89f631b9f9bbe8aedfd0ac Mon Sep 17 00:00:00 2001 From: chasegawa Date: Thu, 9 Sep 2021 22:05:17 -0700 Subject: [PATCH] SHIBUI-1743 Changed group regex validation to expect and use JS style regex --- .../admin/ui/security/model/Group.java | 2 +- .../ui/security/service/GroupServiceImpl.java | 36 ++++++++++++++----- .../EntityDescriptorControllerTests.groovy | 21 ++++++++++- ...ityDescriptorVersionControllerTests.groovy | 2 +- ...cHttpMetadataResolverValidatorTests.groovy | 2 +- ...dHttpMetadataResolverValidatorTests.groovy | 2 +- .../UsersControllerIntegrationTests.groovy | 2 +- .../security/service/GroupServiceTests.groovy | 8 ++--- ...PAEntityDescriptorServiceImplTests2.groovy | 2 +- 9 files changed, 58 insertions(+), 19 deletions(-) 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 7eb8718f3..f90423b66 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,7 +23,7 @@ @EntityListeners(GroupUpdatedEntityListener.class) @Entity(name = "user_groups") public class Group implements Owner { - public static final String DEFAULT_REGEX = "^.+$"; //everything + public static final String DEFAULT_REGEX = "/(?!^()$)^(.*)$/"; //everything except an empty string @Transient @JsonIgnore 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 3a1dfd029..23556bcfe 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 @@ -13,18 +13,24 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import javax.script.ScriptEngine; +import javax.script.ScriptEngineManager; +import javax.script.ScriptException; import java.util.List; -import java.util.regex.Pattern; @Service @NoArgsConstructor public class GroupServiceImpl implements IGroupService { + private static final String CHECK_REGEX = "function isValid(exp){try{new RegExp(exp);return true;}catch(e){return false;}};isValid(rgx);"; + private static final String REGEX_MATCHER = "function validate(r, s){ return (r).test(s);};validate(rgx, str);"; + private final ScriptEngine engine = new ScriptEngineManager().getEngineByName("js"); + @Autowired protected GroupsRepository groupRepository; - + @Autowired protected OwnershipRepository ownershipRepository; - + public GroupServiceImpl(GroupsRepository repo, OwnershipRepository ownershipRepository) { this.groupRepository = repo; this.ownershipRepository = ownershipRepository; @@ -63,8 +69,18 @@ public void deleteDefinition(String resourceId) throws EntityNotFoundException, @Override public boolean doesStringMatchGroupPattern(String groupId, String uri) { Group group = find(groupId); - //@TODO change matching to rhino - return Pattern.matches(group.getValidationRegex(), uri); + + String regExp = group.getValidationRegex(); + engine.put("str", uri); + try { + engine.eval("var rgx=" + regExp); + Object value = engine.eval(REGEX_MATCHER); + return Boolean.valueOf(value.toString()); + } + catch (ScriptException e) { + return false; + } + } @Override @@ -75,7 +91,7 @@ public void ensureAdminGroupExists() { g = new Group(); g.setName("ADMIN-GROUP"); g.setResourceId("admingroup"); - g.setValidationRegex("^.+$"); // Just about everything + g.setValidationRegex(Group.DEFAULT_REGEX); g = groupRepository.save(g); } Group.ADMIN_GROUP = g; @@ -112,9 +128,13 @@ private void validateGroupRegex(Group group) throws InvalidGroupRegexException { return; } try { - Pattern.compile(group.getValidationRegex()); + engine.eval("var rgx=" + group.getValidationRegex()); + Object value = engine.eval(CHECK_REGEX); + if (!Boolean.valueOf(value.toString())) { + throw new InvalidGroupRegexException("Invalid Regular Expression [ " + group.getValidationRegex() + " ]"); + } } - catch (Exception e) { + catch (ScriptException e) { throw new InvalidGroupRegexException("Invalid Regular Expression [ " + group.getValidationRegex() + " ]"); } } 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 e337ad632..89bf72ea6 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 @@ -82,7 +82,7 @@ class EntityDescriptorControllerTests extends AbstractBaseDataJpaTest { Group gb = new Group() gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") - gb.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + gb.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") gb = groupService.createGroup(gb) randomGenerator = new RandomGenerator() @@ -188,6 +188,25 @@ class EntityDescriptorControllerTests extends AbstractBaseDataJpaTest { .andExpect(jsonPath("\$.[1].idOfOwner").value("admingroup")) } + @WithMockUser(value = "someUser", roles = ["USER"]) + def 'POST create new - entity id does not match pattern'() { + when: + def expectedEntityId = 'https://google.com/blah/blah' + EntityDescriptorRepresentation edRep = new EntityDescriptorRepresentation() + edRep.setEntityId(expectedEntityId) + edRep.setServiceProviderName("spName") + + def edRepJson = mapper.writeValueAsString(edRep) + + then: + try { + mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(edRepJson)) + false + } catch (NestedServletException expected) { + expected.getCause() instanceof InvalidPatternMatchException + } + } + @WithMockUser(value = "someUser", roles = ["USER"]) def 'POST create new - verifying validation on entityID and ACS locations'() { given: diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorVersionControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorVersionControllerTests.groovy index 07334aadb..f5714441d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorVersionControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorVersionControllerTests.groovy @@ -78,7 +78,7 @@ class EntityDescriptorVersionControllerTests extends AbstractBaseDataJpaTest { Group gb = new Group() gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") - gb.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + gb.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") gb = groupService.createGroup(gb) controller = new EntityDescriptorController(versionService) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/DynamicHttpMetadataResolverValidatorTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/DynamicHttpMetadataResolverValidatorTests.groovy index 21af72924..5164ae82d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/DynamicHttpMetadataResolverValidatorTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/DynamicHttpMetadataResolverValidatorTests.groovy @@ -27,7 +27,7 @@ class DynamicHttpMetadataResolverValidatorTests extends AbstractBaseDataJpaTest g.setResourceId("shib") g.setName("shib") // This is valid for a url with "shib.org" in it - g.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + g.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") g = groupServiceForTesting.createGroup(g) Optional userRole = roleRepository.findByName("ROLE_USER") diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/FileBackedHttpMetadataResolverValidatorTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/FileBackedHttpMetadataResolverValidatorTests.groovy index c82f0aa18..9881498da 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/FileBackedHttpMetadataResolverValidatorTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/validator/FileBackedHttpMetadataResolverValidatorTests.groovy @@ -22,7 +22,7 @@ class FileBackedHttpMetadataResolverValidatorTests extends AbstractBaseDataJpaTe g.setResourceId("shib") g.setName("shib") // This is valid for a url with "shib.org" in it - g.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + g.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") g = groupService.createGroup(g) Optional userRole = roleRepository.findByName("ROLE_USER") diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy index 6fd839e79..fbd232620 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy @@ -44,7 +44,7 @@ class UsersControllerIntegrationTests extends AbstractBaseDataJpaTest { def users static RESOURCE_URI = '/api/admin/users' - static VALIDATION_REGEX = "^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$" + static VALIDATION_REGEX = "/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/" def setup() { def controller = new UsersController(userRepository, userService) 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 index 2cc9b1968..e71fdeb17 100644 --- 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 @@ -25,7 +25,7 @@ class GroupServiceTests extends AbstractBaseDataJpaTest { g.getValidationRegex() == Group.DEFAULT_REGEX when: - g.setValidationRegex("/*") + g.setValidationRegex("/\\w\\b\\w/") try { g = groupService.updateGroup(g) } catch (Exception shouldNotOccur) { @@ -33,10 +33,10 @@ class GroupServiceTests extends AbstractBaseDataJpaTest { } then: - g.getValidationRegex() == "/*" + g.getValidationRegex() == "/\\w\\b\\w/" when: - g.setValidationRegex("^(http:\\\\/\\\\/www\\\\.|https:\\\\/\\\\/www\\\\.|http:\\\\/\\\\/|https:\\\\/\\\\/)?[a-z0-9]+([\\\\-\\\\.]twitter+)\\\\.[a-z]{2,5}(:[0-9]{1,5})?(\\\\/.*)?\\\$") + g.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") try { g = groupService.updateGroup(g) } catch (Exception shouldNotOccur) { @@ -44,7 +44,7 @@ class GroupServiceTests extends AbstractBaseDataJpaTest { } then: - g.getValidationRegex() == "^(http:\\\\/\\\\/www\\\\.|https:\\\\/\\\\/www\\\\.|http:\\\\/\\\\/|https:\\\\/\\\\/)?[a-z0-9]+([\\\\-\\\\.]twitter+)\\\\.[a-z]{2,5}(:[0-9]{1,5})?(\\\\/.*)?\\\$" + g.getValidationRegex() == "/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/" when: g.setValidationRegex("*") diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy index 22e90bf14..3cc6123c8 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy @@ -23,7 +23,7 @@ class JPAEntityDescriptorServiceImplTests2 extends AbstractBaseDataJpaTest { Group gb = new Group() gb.setResourceId("testingGroupBBB") gb.setName("Group BBB") - gb.setValidationRegex("^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$") + gb.setValidationRegex("/^(?:https?:\\/\\/)?(?:[^.]+\\.)?shib\\.org(\\/.*)?\$/") gb = groupService.createGroup(gb) Optional userRole = roleRepository.findByName("ROLE_USER")