Skip to content

Commit

Permalink
User count metrics for site roles and special groups (#4776)
Browse files Browse the repository at this point in the history
  • Loading branch information
@labkey-jeckels
labkey-jeckels authored Sep 26, 2023
1 parent 9115a1a commit 2979cd8
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 62 deletions.
8 changes: 3 additions & 5 deletions api/src/org/labkey/api/security/RoleAssignment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.labkey.api.security;

import org.jetbrains.annotations.NotNull;
import org.labkey.api.security.roles.Role;

/**
Expand Down Expand Up @@ -77,12 +78,9 @@ public void setRole(Role role)
}

@Override
public int compareTo(RoleAssignment other)
public int compareTo(@NotNull RoleAssignment other)
{
if(null == other)
throw new NullPointerException();

int ret = 0;
int ret;
//sort by resource id, then user id, then role unique name
//FIX: 10023 -- the resource ids should never be null, but some modules
//seem to use null resource ids on occasion (esp. reports and pipe roots)
Expand Down
87 changes: 61 additions & 26 deletions api/src/org/labkey/api/security/SecurityManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.labkey.api.security;

import org.apache.commons.codec.binary.Base64;
import org.apache.commons.collections4.bag.HashBag;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -80,6 +81,7 @@
import org.labkey.api.settings.LenientStartupPropertyHandler;
import org.labkey.api.settings.StartupProperty;
import org.labkey.api.settings.StartupPropertyEntry;
import org.labkey.api.usageMetrics.UsageMetricsProvider;
import org.labkey.api.util.ConfigurationException;
import org.labkey.api.util.DateUtil;
import org.labkey.api.util.GUID;
Expand All @@ -94,7 +96,6 @@
import org.labkey.api.util.SessionHelper;
import org.labkey.api.util.TestContext;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.emailTemplate.EmailTemplate;
import org.labkey.api.util.emailTemplate.EmailTemplateService;
import org.labkey.api.util.emailTemplate.UserOriginatedEmailTemplate;
import org.labkey.api.util.logging.LogHelper;
Expand Down Expand Up @@ -182,6 +183,42 @@ public class SecurityManager
EmailTemplateService.get().registerTemplate(PasswordResetAdminEmailTemplate.class);
}

public static UsageMetricsProvider getMetricsProvider()
{
return () -> {
Map<String, Object> result = new HashMap<>();

Map<String, Integer> roleCounts = new HashMap<>();
Set<Role> siteRoles = RoleManager.getSiteRoles();
for (RoleAssignment assignment : ContainerManager.getRoot().getPolicy().getAssignments())
{
Role role = assignment.getRole();
int count = roleCounts.getOrDefault(role.getName(), 0);
if (siteRoles.contains(role))
{
UserPrincipal principal = SecurityManager.getPrincipal(assignment.getUserId());
if (principal != null && principal.isActive())
{
switch (principal.getPrincipalType())
{
case USER -> count++;
case GROUP -> count += getAllGroupMembers((Group)principal, MemberType.ACTIVE_USERS).size();
}
}
}
roleCounts.put(role.getName(), count);
}
result.put("SiteRoleUserCounts", roleCounts);

Map<String, Integer> groupCounts = new HashMap<>();
groupCounts.put("SiteAdmin", getAllGroupMembers(getGroup(Group.groupAdministrators), MemberType.ACTIVE_USERS).size());
groupCounts.put("Developer", getAllGroupMembers(getGroup(Group.groupDevelopers), MemberType.ACTIVE_USERS).size());
result.put("SiteGroupUserCounts", groupCounts);

return result;
};
}

public enum PermissionSet
{
ADMIN("Admin (all permissions)", ACL.PERM_ALLOWALL),
Expand Down Expand Up @@ -1843,10 +1880,10 @@ public static Map<UserPrincipal, List<UserPrincipal>> getRedundantGroupMembers(G
Map<UserPrincipal, List<UserPrincipal>> redundantMembers = new HashMap<>();
Set<UserPrincipal> origMembers = getGroupMembers(group, MemberType.ALL_GROUPS_AND_USERS);
LinkedList<UserPrincipal> visited = new LinkedList<>();
for (UserPrincipal memberGroup : getGroupMembers(group, MemberType.GROUPS))
for (Group memberGroup : getGroupMembers(group, MemberType.GROUPS))
{
visited.addLast(memberGroup);
checkForRedundantMembers((Group)memberGroup, origMembers, redundantMembers, visited);
checkForRedundantMembers(memberGroup, origMembers, redundantMembers, visited);
visited.removeLast();
}
return redundantMembers;
Expand Down Expand Up @@ -1916,13 +1953,13 @@ private static void checkForMembership(UserPrincipal principal, Group group, Lin
}
}

for (UserPrincipal member : SecurityManager.getGroupMembers(group, MemberType.GROUPS))
for (Group member : SecurityManager.getGroupMembers(group, MemberType.GROUPS))
{
if (visited.contains(member) || member.equals(principal))
continue;

visited.addLast(member);
checkForMembership(principal, (Group)member, visited, memberships);
checkForMembership(principal, member, visited, memberships);
visited.removeLast();
}
}
Expand All @@ -1945,14 +1982,14 @@ public static String getMembershipPathwayHTMLDisplay(Set<List<UserPrincipal>> pa
for (List<UserPrincipal> path : paths)
{
// add an extra line if there are > 1 paths displayed
if (sb.length() > 0)
if (!sb.isEmpty())
sb.append("<BR/>");

StringBuilder spacer = new StringBuilder();
for (int i = path.size()-1; i > 0 ; i--)
{
String beginTxt = (i == path.size()-1 ? PageFlowUtil.filter(userDisplay) : "Which");
sb.append(spacer.toString()).append(beginTxt).append(" is a member of ").append("<strong>").append(PageFlowUtil.filter(path.get(i-1).getName())).append("</strong>");
sb.append(spacer).append(beginTxt).append(" is a member of ").append("<strong>").append(PageFlowUtil.filter(path.get(i-1).getName())).append("</strong>");
sb.append("<BR/>");
spacer.append("&nbsp;&nbsp;&nbsp;");
}
Expand Down Expand Up @@ -2032,7 +2069,7 @@ public static Collection<Integer> getFolderUserids(Container c)

//don't filter if all site users is playing a role
Group allSiteUsers = SecurityManager.getGroup(Group.groupUsers);
if (policy.getAssignedRoles(allSiteUsers).size() != 0)
if (!policy.getAssignedRoles(allSiteUsers).isEmpty())
{
// Just select all users
SQLFragment sql = new SQLFragment("SELECT u.UserId FROM ");
Expand Down Expand Up @@ -2304,7 +2341,7 @@ public static List<ValidEmail> normalizeEmails(String[] rawEmails, List<String>

public static List<ValidEmail> normalizeEmails(List<String> rawEmails, List<String> invalidEmails)
{
if (rawEmails == null || rawEmails.size() == 0)
if (rawEmails == null || rawEmails.isEmpty())
return Collections.emptyList();

List<ValidEmail> emails = new ArrayList<>(rawEmails.size());
Expand All @@ -2330,9 +2367,9 @@ public static SecurityMessage getRegistrationMessage(String mailPrefix, boolean
{
SecurityMessage sm = new SecurityMessage(isAdminCopy);
Class<? extends RegistrationEmailTemplate> templateClass = isAdminCopy ? RegistrationAdminEmailTemplate.class : RegistrationEmailTemplate.class;
EmailTemplate et = EmailTemplateService.get().getEmailTemplate(templateClass);
SecurityEmailTemplate et = EmailTemplateService.get().getEmailTemplate(templateClass);
sm.setMessagePrefix(mailPrefix);
sm.setEmailTemplate((SecurityEmailTemplate)et);
sm.setEmailTemplate(et);
sm.setType("User Registration Email");

return sm;
Expand Down Expand Up @@ -2362,8 +2399,8 @@ private static SecurityMessage getDefaultResetMessage(boolean isAdminCopy)
{
SecurityMessage sm = new SecurityMessage(isAdminCopy);
Class<? extends PasswordResetEmailTemplate> templateClass = isAdminCopy ? PasswordResetAdminEmailTemplate.class : PasswordResetEmailTemplate.class;
EmailTemplate et = EmailTemplateService.get().getEmailTemplate(templateClass);
sm.setEmailTemplate((SecurityEmailTemplate)et);
SecurityEmailTemplate et = EmailTemplateService.get().getEmailTemplate(templateClass);
sm.setEmailTemplate(et);
sm.setType("Reset Password Email");
return sm;
}
Expand Down Expand Up @@ -3177,7 +3214,7 @@ public static boolean hasPermissions(@Nullable String logMsg, SecurityPolicy pol

var granted = getPermissions(policy, principal, contextualRoles);
boolean ret = opt.accept(granted, permissions);
SecurityLogger.log("SecurityPolicy.hasPermissions " + permissions.toString(), principal, policy, ret);
SecurityLogger.log("SecurityPolicy.hasPermissions " + permissions, principal, policy, ret);

return ret;
}
Expand Down Expand Up @@ -3220,7 +3257,7 @@ public static Set<Role> getEffectiveRoles(@NotNull SecurityPolicy policy, @NotNu
Set<Role> roles = policy.getRoles(principal.getGroups());
roles.addAll(policy.getAssignedRoles(principal));
if (includeContextualRoles)
roles.addAll(principal.getContextualRoles(policy));;
roles.addAll(principal.getContextualRoles(policy));
return roles;
}

Expand All @@ -3244,9 +3281,7 @@ boolean accept(Set<Class<? extends Permission>> granted, Set<Class<? extends Per
};

abstract boolean accept(Set<Class<? extends Permission>> granted, Set<Class<? extends Permission>> required);
};


}


public static class TestCase extends Assert
Expand Down Expand Up @@ -3594,7 +3629,7 @@ public void testEmailValidation()

String defaultDomain = ValidEmail.getDefaultDomain();
// If default domain is defined this should succeed; if it's not defined, this should fail.
testEmail("foo", defaultDomain.length() > 0);
testEmail("foo", !defaultDomain.isEmpty());

testEmail("~()@bar.com", false);
testEmail("[email protected]@con", false);
Expand Down Expand Up @@ -3753,7 +3788,7 @@ public boolean performChecks()
public void testGetUsersWithPermissions() throws Exception
{
Container parent = JunitUtil.getTestContainer();
Container test = null;
Container test;
Pair<ValidEmail,User> userAB = new Pair<>(new ValidEmail("[email protected]"), null);
Pair<ValidEmail,User> userAC = new Pair<>(new ValidEmail("[email protected]"), null);
Pair<ValidEmail,User> userABC = new Pair<>(new ValidEmail("[email protected]"), null);
Expand Down Expand Up @@ -3791,25 +3826,25 @@ public boolean isForbiddenProject(User user)
policy.addAssignment(new RoleAssignment(id,userABC.getValue(),new RoleAB()));
policy.addAssignment(new RoleAssignment(id,userABC.getValue(),new RoleAC()));

var usersWithAll = new HashSet(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionA.class)));
var usersWithAll = new HashSet<>(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionA.class)));
assertEquals(3, usersWithAll.size());
assertTrue(usersWithAll.contains(userAB.getValue()));
assertTrue(usersWithAll.contains(userAC.getValue()));
assertTrue(usersWithAll.contains(userABC.getValue()));
var usersWithAny = new HashSet(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionA.class)));
var usersWithAny = new HashSet<>(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionA.class)));
assertEquals(usersWithAll, usersWithAny);

usersWithAll = new HashSet(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionB.class)));
usersWithAll = new HashSet<>(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionB.class)));
assertEquals(2, usersWithAll.size());
assertTrue(usersWithAll.contains(userAB.getValue()));
assertTrue(usersWithAll.contains(userABC.getValue()));
usersWithAny = new HashSet(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionB.class)));
usersWithAny = new HashSet<>(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionB.class)));
assertEquals(usersWithAll, usersWithAny);

usersWithAll = new HashSet(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionB.class, PermissionC.class)));
usersWithAll = new HashSet<>(SecurityManager.getUsersWithPermissions(test, Set.of(PermissionB.class, PermissionC.class)));
assertEquals(1, usersWithAll.size());
assertTrue(usersWithAll.contains(userABC.getValue()));
usersWithAny = new HashSet(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionB.class, PermissionC.class)));
usersWithAny = new HashSet<>(SecurityManager.getUsersWithOneOf(test, Set.of(PermissionB.class, PermissionC.class)));
assertEquals(3, usersWithAny.size());
assertTrue(usersWithAny.contains(userAB.getValue()));
assertTrue(usersWithAny.contains(userAC.getValue()));
Expand Down
16 changes: 3 additions & 13 deletions api/src/org/labkey/api/security/User.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.json.JSONString;
import org.labkey.api.attachments.Attachment;
import org.labkey.api.attachments.AttachmentService;
import org.labkey.api.compliance.ComplianceFolderSettings;
import org.labkey.api.compliance.ComplianceService;
import org.labkey.api.data.Container;
import org.labkey.api.data.ContainerManager;
Expand Down Expand Up @@ -54,11 +53,8 @@
import org.labkey.api.view.ActionURL;

import java.io.Serializable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -349,14 +345,8 @@ public boolean isDeveloper()
return isAllowedGlobalRoles() && hasRootPermission(PlatformDeveloperPermission.class);
}

static final Set<Class<? extends Permission>> trustedanalyst = Collections.unmodifiableSet(new HashSet<Class<? extends Permission>>(Arrays.asList(
AnalystPermission.class,
TrustedPermission.class
)));
static final Set<Class<? extends Permission>> trustedbrowserdev = Collections.unmodifiableSet(new HashSet<Class<? extends Permission>>(Arrays.asList(
BrowserDeveloperPermission.class,
TrustedPermission.class
)));
static final Set<Class<? extends Permission>> trustedanalyst = Set.of(AnalystPermission.class, TrustedPermission.class);
static final Set<Class<? extends Permission>> trustedbrowserdev = Set.of(BrowserDeveloperPermission.class, TrustedPermission.class);

// NOTE all PlatformDeveloper are TrustedAnalyst and all TrustedAnalyst are TrustedBrowserDev
// Usually you should only have one of these tests
Expand Down Expand Up @@ -687,7 +677,7 @@ public static JSONObject getUserProps(User user, User currentUser, @Nullable Con
props.put("isSystem", user.isSystem());

// PHI level
/** CONSIDER: Only include maxAllowedPhi if {@link ComplianceFolderSettings#isPhiRolesRequired()} */
// CONSIDER: Only include maxAllowedPhi if {@link ComplianceFolderSettings#isPhiRolesRequired()}
if (nonNullContainer)
{
PHI maxAllowedPhi = ComplianceService.get().getMaxAllowedPhi(container, user);
Expand Down
Loading

0 comments on commit 2979cd8

Please sign in to comment.