Skip to content

Commit

Permalink
Merge pull request #7351 from poikilotherm/7344-mailgroup-regex
Browse files Browse the repository at this point in the history
7344 mailgroup regex
  • Loading branch information
kcondon authored Oct 28, 2020
2 parents 9b987f6 + 52f140c commit 666768a
Show file tree
Hide file tree
Showing 11 changed files with 186 additions and 26 deletions.
26 changes: 26 additions & 0 deletions doc/sphinx-guides/source/admin/mail-groups.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,32 @@ To load it into your Dataverse installation, either use a ``POST`` or ``PUT`` re

``curl -X POST -H 'Content-type: application/json' http://localhost:8080/api/admin/groups/domain --upload-file domainGroup1.json``

Matching with Domains or Regular Expressions
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Adding simple domain names requires exact matches of user email domains and the configured domains of a group. Although you could add multiple domains to a group, those still require exact matches.

You can also use one or multiple regular expressions instead of simple domains for a group. Those should not be mixed, although it would work. Regular expressions still require exact matches, but are much more flexible and are designed to support installation-specific use cases for group management.

Some hints:

- Due to their excessive CPU usage, regular expressions should be used rarely.
- Remember to properly escape "\" in your regular expression. Both Java and JSON are a bit picky about this. E.g. a character class "\d" would have to be escaped as "\\d". Plenty of tutorials on the web explain this in more detail.
- There is no way Dataverse can detect a wrong regular expression for you. Be sure to do extensive testing, as a misconfigured group could result in privilege escalation or an unexpected influx of support contacts.
- Remember to enable the regular expression support for a group by adding ``"regex": true``!

A short example for a group using regular expressions:

.. code-block:: json
{
"name": "Users from @example.org",
"alias": "exampleorg-regex",
"description": "Any verified user from x@example.org or x@sub.example.org will be included.",
"regex": true,
"domains": ["example\\.org", "[a-z]+\\.example\\.org"]
}
Updating a Mail Domain Group
----------------------------

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/api/Groups.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ public Response deleteShibGroup( @PathParam("primaryKey") String id ) {
public Response createMailDomainGroup(JsonObject dto) throws JsonParseException {
MailDomainGroup grp = new JsonParser().parseMailDomainGroup(dto);
mailDomainGroupPrv.saveOrUpdate(Optional.empty(), grp);
mailDomainGroupPrv.updateGroups();

return created("/groups/domain/" + grp.getPersistedGroupAlias(), json(grp) );
}
Expand All @@ -254,6 +255,7 @@ public Response updateMailDomainGroups(@PathParam("groupAlias") String groupAlia

MailDomainGroup grp = new JsonParser().parseMailDomainGroup(dto);
mailDomainGroupPrv.saveOrUpdate(Optional.of(groupAlias), grp);
mailDomainGroupPrv.updateGroups();

return created("/groups/domain/" + grp.getPersistedGroupAlias(), json(grp) );
}
Expand All @@ -276,6 +278,7 @@ public Response getMailDomainGroup(@PathParam("groupAlias") String groupAlias) {
@Path("domain/{groupAlias}")
public Response deleteMailDomainGroup(@PathParam("groupAlias") String groupAlias) {
mailDomainGroupPrv.delete(groupAlias);
mailDomainGroupPrv.updateGroups();
return ok("Group " + groupAlias + " deleted.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import edu.harvard.iq.dataverse.authorization.groups.impl.PersistedGlobalGroup;
import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
import org.hibernate.validator.constraints.NotEmpty;

import java.util.Arrays;
import java.util.List;
Expand All @@ -11,6 +10,7 @@
import javax.persistence.NamedQueries;
import javax.persistence.NamedQuery;
import javax.persistence.Transient;
import javax.validation.constraints.NotEmpty;
;

/**
Expand All @@ -32,6 +32,8 @@ public class MailDomainGroup extends PersistedGlobalGroup {
@NotEmpty
private String emailDomains;

private boolean isRegEx = false;

@Transient
private MailDomainGroupProvider provider;

Expand All @@ -54,6 +56,13 @@ public List<String> getEmailDomainsAsList() {
return Arrays.asList(this.emailDomains.split(";"));
}

public boolean isRegEx() {
return isRegEx;
}
public void setIsRegEx(boolean isRegEx) {
this.isRegEx = isRegEx;
}

@Override
public MailDomainGroupProvider getGroupProvider() {
return provider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ public void delete(String groupAlias) {
emailGroupSvc.delete(groupAlias);
}

/**
* Enable triggering an update of groups from the database. Used in the API to refresh after changes.
*/
public void updateGroups() {
emailGroupSvc.updateGroups();
}

/**
* Sets the provider of the passed explicit group to {@code this}.
* @param eg the collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,13 @@
import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean;
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
import edu.harvard.iq.dataverse.confirmemail.ConfirmEmailServiceBean;
import org.ocpsoft.rewrite.config.Not;

import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.PostConstruct;
import javax.ejb.Stateless;
import javax.ejb.*;
import javax.inject.Inject;
import javax.inject.Named;
import javax.persistence.EntityManager;
Expand All @@ -28,7 +23,9 @@
* Also containing the business logic to decide about matching groups.
*/
@Named
@Stateless
@Singleton
@Startup
@DependsOn("StartupFlywayMigrator")
public class MailDomainGroupServiceBean {

private static final Logger logger = Logger.getLogger(edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroupServiceBean.class.getName());
Expand All @@ -42,12 +39,32 @@ public class MailDomainGroupServiceBean {
ActionLogServiceBean actionLogSvc;

MailDomainGroupProvider provider;
List<MailDomainGroup> simpleGroups = Collections.EMPTY_LIST;
Map<MailDomainGroup, Pattern> regexGroups = new HashMap<>();

@PostConstruct
void setup() {
provider = new MailDomainGroupProvider(this);
this.updateGroups();
}

/**
* Update the groups from the database.
* This is done because regex compilation is an expensive operation and should be cached.
*/
@Lock(LockType.WRITE)
public void updateGroups() {
List<MailDomainGroup> all = findAll();
this.simpleGroups = all.stream().filter(mg -> !mg.isRegEx()).collect(Collectors.toList());
this.regexGroups = all.stream()
.filter(MailDomainGroup::isRegEx)
.collect(Collectors.toMap(
mg -> mg,
mg -> Pattern.compile(mg.getEmailDomains().replace(";","|"))
));
}

@Lock(LockType.READ)
public MailDomainGroupProvider getProvider() {
return provider;
}
Expand All @@ -57,6 +74,7 @@ public MailDomainGroupProvider getProvider() {
* @param user
* @return A collection of groups with matching email domains
*/
@Lock(LockType.READ)
public Set<MailDomainGroup> findAllWithDomain(AuthenticatedUser user) {

// if the mail address is not verified, escape...
Expand All @@ -69,12 +87,17 @@ public Set<MailDomainGroup> findAllWithDomain(AuthenticatedUser user) {
// transform to lowercase, in case someone uses uppercase letters. (we store the comparison values in lowercase)
String domain = oDomain.get().toLowerCase();

// get all groups and filter
List<MailDomainGroup> rs = em.createNamedQuery("MailDomainGroup.findAll", MailDomainGroup.class).getResultList();

return rs.stream()
.filter(mg -> mg.getEmailDomainsAsList().contains(domain))
.collect(Collectors.toSet());
// scan simple groups (containing an exact match of the domain)
Set<MailDomainGroup> result = this.simpleGroups.stream()
.filter(mg -> mg.getEmailDomainsAsList().contains(domain))
.collect(Collectors.toSet());
// scan regex based groups (domain matching a regular expression)
result.addAll(this.regexGroups.keySet().stream()
.filter(MailDomainGroup::isRegEx)
.filter(mg -> regexGroups.get(mg).matcher(domain).matches())
.collect(Collectors.toSet()));
return result;

}
return Collections.emptySet();
}
Expand All @@ -83,6 +106,7 @@ public Set<MailDomainGroup> findAllWithDomain(AuthenticatedUser user) {
* Get all mail domain groups from the database.
* @return A result list from the database. May be null if no results found.
*/
@Lock(LockType.READ)
public List<MailDomainGroup> findAll() {
return em.createNamedQuery("MailDomainGroup.findAll", MailDomainGroup.class).getResultList();
}
Expand All @@ -92,6 +116,7 @@ public List<MailDomainGroup> findAll() {
* @param groupAlias
* @return
*/
@Lock(LockType.READ)
Optional<MailDomainGroup> findByAlias(String groupAlias) {
try {
return Optional.of(
Expand All @@ -112,6 +137,7 @@ Optional<MailDomainGroup> findByAlias(String groupAlias) {
* @return The saved entity, including updated group provider attribute
* @throws NotFoundException if groupName does not match both a group in database and the alias of the provided group
*/
@Lock(LockType.WRITE)
public MailDomainGroup saveOrUpdate(Optional<String> groupAlias, MailDomainGroup grp ) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.GlobalGroups, "mailDomainCreate");
alr.setInfo(grp.getIdentifier());
Expand Down Expand Up @@ -149,6 +175,7 @@ public MailDomainGroup saveOrUpdate(Optional<String> groupAlias, MailDomainGroup
* @param groupAlias
* @throws NotFoundException if no group with given groupAlias exists.
*/
@Lock(LockType.WRITE)
public void delete(String groupAlias) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.GlobalGroups, "mailDomainDelete");
alr.setInfo(groupAlias);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,19 @@ public MailDomainGroup parseMailDomainGroup(JsonObject obj) throws JsonParseExce
grp.setDisplayName(getMandatoryString(obj, "name"));
grp.setDescription(obj.getString("description", null));
grp.setPersistedGroupAlias(getMandatoryString(obj, "alias"));
grp.setIsRegEx(obj.getBoolean("regex", false));
if ( obj.containsKey("domains") ) {
List<String> domains =
Optional.ofNullable(obj.getJsonArray("domains"))
.orElse(Json.createArrayBuilder().build())
.getValuesAs(JsonString.class)
.stream()
.map(JsonString::getString)
.filter(d -> DomainValidator.getInstance().isValid(d))
// only validate if this group hasn't regex support enabled
.filter(d -> (grp.isRegEx() || DomainValidator.getInstance().isValid(d)))
.collect(Collectors.toList());
if (domains.isEmpty())
throw new JsonParseException("Field domains may not be an empty array or contain invalid domains.");
throw new JsonParseException("Field domains may not be an empty array or contain invalid domains. Enabled regex support?");
grp.setEmailDomains(domains);
} else {
throw new JsonParseException("Field domains is mandatory.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ public static JsonObjectBuilder json(MailDomainGroup grp) {
.add("id", grp.getId() )
.add("name", grp.getDisplayName() )
.add("description", grp.getDescription() )
.add("domains", asJsonArray(grp.getEmailDomainsAsList()) );
.add("domains", asJsonArray(grp.getEmailDomainsAsList()) )
.add("regex", grp.isRegEx());
return bld;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE persistedglobalgroup ADD COLUMN IF NOT EXISTS isRegEx BOOLEAN NOT NULL DEFAULT FALSE;
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,78 @@ void setup() {
}

@Test
void testFindWithVerifiedEmail() {
void testUpdateGroup() {
// given
List<MailDomainGroup> db = new ArrayList<>(Arrays.asList(MailDomainGroupTest.genGroup(),MailDomainGroupTest.genGroup(),MailDomainGroupTest.genGroup()));
List<MailDomainGroup> db = new ArrayList<>(Arrays.asList(MailDomainGroupTest.genGroup(), MailDomainGroupTest.genGroup(), MailDomainGroupTest.genRegexGroup()));
mockQuery("MailDomainGroup.findAll", db);

// when
svc.updateGroups();

// then
assertEquals(2, svc.simpleGroups.size());
assertEquals(1, svc.regexGroups.size());
}

@Test
void testUpdateGroupMultiRegex() {
// given
List<MailDomainGroup> db = new ArrayList<>(Arrays.asList(MailDomainGroupTest.genGroup(), MailDomainGroupTest.genRegexGroup()));
MailDomainGroup longRegex = MailDomainGroupTest.genRegexGroup();
longRegex.setEmailDomains(Arrays.asList("example\\.org", ".+\\.example\\.org"));
db.add(longRegex);
mockQuery("MailDomainGroup.findAll", db);

// when
svc.updateGroups();

// then
assertEquals(1, svc.simpleGroups.size());
assertEquals(2, svc.regexGroups.size());
assertEquals("example\\.org|.+\\.example\\.org", svc.regexGroups.get(longRegex).pattern());
}

private static Stream<Arguments> mailTestExamples() {
return Stream.of(
Arguments.of("simpleGroupOnly@foobar.com", Arrays.asList("foobar.com"), false, false),
Arguments.of("regexButNotActive@example.org", Arrays.asList("^example\\.org$"), false, true),
Arguments.of("singleRegexMatch@example.org", Arrays.asList("^example\\.org$"), true, false),
Arguments.of("singleRegexFail-1@hello.example.org", Arrays.asList("^example\\.org$"), true, true),
Arguments.of("singleRegexFail-2@foobar.com", Arrays.asList("^example\\.org$"), true, true),
Arguments.of("multiRegexMatch-1@example.org", Arrays.asList("^example\\.org$", "^.+\\.example\\.org$"), true, false),
Arguments.of("multiRegexMatch-2@hello.example.org", Arrays.asList("^example\\.org$", "^.+\\.example\\.org$"), true, false),
Arguments.of("multiRegexFail@hfoobar.com", Arrays.asList("^example\\.org$", "^.+\\.example\\.org$"), true, true),
Arguments.of("invalidRegex@hfoobar.com", Arrays.asList("$foobar.com$"), true, true),
Arguments.of("noOvermatchingRegex@hfoobar.com.eu", Arrays.asList("bar\\.com"), true, true)
);
}

@ParameterizedTest
@MethodSource("mailTestExamples")
void testFindWithVerifiedEmail(String email, List<String> domains, boolean isRegex, boolean shouldBeEmpty) {
// given
List<MailDomainGroup> db = new ArrayList<>(Arrays.asList(MailDomainGroupTest.genGroup()));
MailDomainGroup test = MailDomainGroupTest.genGroup();
test.setEmailDomains("foobar.com");
test.setEmailDomains(domains);
test.setIsRegEx(isRegex);
db.add(test);
mockQuery("MailDomainGroup.findAll", db);
svc.updateGroups();

AuthenticatedUser u = new AuthenticatedUser();
u.setEmail("test@foobar.com");
u.setEmail(email);
when(confirmEmailSvc.hasVerifiedEmail(u)).thenReturn(true);

// when
Set<MailDomainGroup> result = svc.findAllWithDomain(u);

// then
Set<MailDomainGroup> expected = new HashSet<>(Arrays.asList(test));
assertEquals(expected, result);
if (shouldBeEmpty) {
assertTrue(result.isEmpty());
} else {
Set<MailDomainGroup> expected = new HashSet<>(Arrays.asList(test));
assertTrue(result.containsAll(expected));
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,11 @@ public static MailDomainGroup genGroup() {
t.setEmailDomains(RandomStringUtils.randomAlphanumeric(5)+".com;"+RandomStringUtils.randomAlphanumeric(5)+".co.uk");
return t;
}

public static MailDomainGroup genRegexGroup() {
MailDomainGroup t = genGroup();
t.setEmailDomains(".+\\.com$");
t.setIsRegEx(true);
return t;
}
}
Loading

0 comments on commit 666768a

Please sign in to comment.