From c1e8ea18abfb279eaaed2101f9030c52863d04b9 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 14 Jul 2025 14:52:35 -0700 Subject: [PATCH 1/7] Issue 53243: stream response from security-getGroupPerms.api --- .../org/labkey/api/action/ApiJsonWriter.java | 5 ++ .../core/security/SecurityApiActions.java | 53 ++++++++++--------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/api/src/org/labkey/api/action/ApiJsonWriter.java b/api/src/org/labkey/api/action/ApiJsonWriter.java index f98a4e23ade..c284f71a068 100644 --- a/api/src/org/labkey/api/action/ApiJsonWriter.java +++ b/api/src/org/labkey/api/action/ApiJsonWriter.java @@ -295,6 +295,11 @@ public void startObject(String name) throws IOException jg.writeObjectFieldStart(name); } + public void startObject() throws IOException + { + jg.writeStartObject(); + } + public void endObject() throws IOException { jg.writeEndObject(); diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index f60543a0206..99ba350d7c4 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -19,6 +19,7 @@ import org.apache.commons.lang3.StringUtils; import org.json.JSONObject; import org.junit.Test; +import org.labkey.api.action.ApiJsonWriter; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; import org.labkey.api.action.ApiVersion; @@ -85,6 +86,7 @@ import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Date; @@ -121,54 +123,53 @@ public void setIncludeSubfolders(boolean includeSubfolders) public static class GetGroupPermsAction extends ReadOnlyApiAction { @Override - public ApiResponse execute(GetGroupPermsForm form, BindException errors) + public ApiResponse execute(GetGroupPermsForm form, BindException errors) throws IOException { + // Issue 53243: stream response from security-getGroupPerms.api Container container = getContainer(); + ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse()); - ApiSimpleResponse response = new ApiSimpleResponse(); + writer.startResponse(); - //if the container is not the root container, get the set of groups - //from the container's project and pass that down the recursion stack - response.put("container", getContainerPerms(container, - SecurityManager.getGroups(container.getProject(), true), - form.isIncludeSubfolders())); + writer.startObject("container"); + writeContainer(writer, container, SecurityManager.getGroups(container.getProject(), true), form.isIncludeSubfolders()); + writer.endObject(); - return response; + writer.endResponse(); + + return null; } - protected Map getContainerPerms(Container container, List groups, boolean recurse) + private void writeContainer(ApiJsonWriter writer, Container container, List groups, boolean includeSubfolders) throws IOException { - Map containerPerms = new HashMap<>(); - containerPerms.put("path", container.getPath()); - containerPerms.put("id", container.getId()); - containerPerms.put("name", container.getName()); - containerPerms.put("isInheritingPerms", container.isInheritedAcl()); - containerPerms.put("groups", getGroupPerms(container, groups)); + writer.writeProperty("path", container.getPath()); + writer.writeProperty("id", container.getId()); + writer.writeProperty("name", container.getName()); + writer.writeProperty("isInheritingPerms", container.isInheritedAcl()); - if (recurse && container.hasChildren()) + if (groups != null && !groups.isEmpty()) + writer.writeProperty("groups", getGroupPerms(container, groups)); + + if (includeSubfolders && container.hasChildren()) { - List> childPerms = new ArrayList<>(); + writer.startList("children"); + for (Container child : container.getChildren()) { if (child.hasPermission(getUser(), ReadPermission.class)) { - childPerms.add(getContainerPerms(child, - child.isProject() ? SecurityManager.getGroups(child, true) : groups, - recurse)); + writer.startObject(); + writeContainer(writer, child, child.isProject() ? SecurityManager.getGroups(child, true) : groups, includeSubfolders); + writer.endObject(); } } - containerPerms.put("children", childPerms); + writer.endList(); } - - return containerPerms; } protected List> getGroupPerms(Container container, List groups) { - if (null == groups) - return null; - List> groupsPerms = new ArrayList<>(); for (Group group : groups) From c34304bb665882831d92dd3fbde23868f25a9536 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 14 Jul 2025 16:34:17 -0700 Subject: [PATCH 2/7] Add includeEmptyPerms flag --- .../core/security/SecurityApiActions.java | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 99ba350d7c4..9eda12ed2ed 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -106,8 +106,19 @@ public class SecurityApiActions { public static class GetGroupPermsForm { + private boolean _includeEmptyPerms = true; private boolean _includeSubfolders = false; + public boolean isIncludeEmptyPerms() + { + return _includeEmptyPerms; + } + + public void setIncludeEmptyPerms(boolean includeEmptyPerms) + { + _includeEmptyPerms = includeEmptyPerms; + } + public boolean isIncludeSubfolders() { return _includeSubfolders; @@ -132,7 +143,7 @@ public ApiResponse execute(GetGroupPermsForm form, BindException errors) throws writer.startResponse(); writer.startObject("container"); - writeContainer(writer, container, SecurityManager.getGroups(container.getProject(), true), form.isIncludeSubfolders()); + writeContainer(writer, container, SecurityManager.getGroups(container.getProject(), true), form.isIncludeSubfolders(), form.isIncludeEmptyPerms()); writer.endObject(); writer.endResponse(); @@ -140,15 +151,13 @@ public ApiResponse execute(GetGroupPermsForm form, BindException errors) throws return null; } - private void writeContainer(ApiJsonWriter writer, Container container, List groups, boolean includeSubfolders) throws IOException + private void writeContainer(ApiJsonWriter writer, Container container, List groups, boolean includeSubfolders, boolean includeEmptyPerms) throws IOException { writer.writeProperty("path", container.getPath()); writer.writeProperty("id", container.getId()); writer.writeProperty("name", container.getName()); writer.writeProperty("isInheritingPerms", container.isInheritedAcl()); - - if (groups != null && !groups.isEmpty()) - writer.writeProperty("groups", getGroupPerms(container, groups)); + writer.writeProperty("groups", getGroupPerms(container, groups, includeEmptyPerms)); if (includeSubfolders && container.hasChildren()) { @@ -159,7 +168,7 @@ private void writeContainer(ApiJsonWriter writer, Container container, List> getGroupPerms(Container container, List groups) + protected List> getGroupPerms(Container container, List groups, boolean includeEmptyPerms) { List> groupsPerms = new ArrayList<>(); for (Group group : groups) { + List effectivePermissions = SecurityManager.getPermissionNames(container, group); + if (effectivePermissions.isEmpty() && !includeEmptyPerms) + continue; + Map groupPerms = new HashMap<>(); groupPerms.put("id", group.getUserId()); groupPerms.put("name", SecurityManager.getDisambiguatedGroupName(group)); groupPerms.put("type", group.getPrincipalType().getTypeChar()); groupPerms.put("isSystemGroup", group.isSystemGroup()); groupPerms.put("isProjectGroup", group.isProjectGroup()); + groupsPerms.add(groupPerms); - //add effective roles - List effectiveRoleList = SecurityManager.getEffectiveRoles(container, group).map(Role::getUniqueName).toList(); - groupPerms.put("roles", effectiveRoleList); - groupPerms.put("effectivePermissions", SecurityManager.getPermissionNames(container, group)); + List roles = SecurityManager.getEffectiveRoles(container, group).map(Role::getUniqueName).toList(); + groupPerms.put("roles", roles); + groupPerms.put("effectivePermissions", effectivePermissions); if (container.hasPermission(getUser(), AdminPermission.class)) { - List> parentGroupInfos = new ArrayList<>(); + List> parentGroups = new ArrayList<>(); group.getGroups().stream().forEach(parentGroupId -> { Group parentGroup = SecurityManager.getGroup(parentGroupId); if (parentGroup != null && parentGroup.getUserId() != group.getUserId()) { - parentGroupInfos.add(getGroupMap(parentGroup)); + parentGroups.add(getGroupMap(parentGroup)); } }); - groupPerms.put("groups", parentGroupInfos); + groupPerms.put("groups", parentGroups); } - - groupsPerms.add(groupPerms); } return groupsPerms; From 0326fea4398ae059582a3ef74eb7a6437a6031ac Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 14 Jul 2025 16:35:15 -0700 Subject: [PATCH 3/7] Change scope of isAdmin check (significant performance gains) --- core/src/org/labkey/core/security/SecurityApiActions.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 9eda12ed2ed..166bd857b6c 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -180,6 +180,7 @@ private void writeContainer(ApiJsonWriter writer, Container container, List> getGroupPerms(Container container, List groups, boolean includeEmptyPerms) { List> groupsPerms = new ArrayList<>(); + boolean isAdmin = container.hasPermission(getUser(), AdminPermission.class); for (Group group : groups) { @@ -199,7 +200,7 @@ protected List> getGroupPerms(Container container, List> parentGroups = new ArrayList<>(); group.getGroups().stream().forEach(parentGroupId -> { From b40b5d37b4ace2ef06ffc9e443b0c2807edd235f Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 16 Jul 2025 16:18:18 -0700 Subject: [PATCH 4/7] Issue 53243: add "includeEmptyPermGroups" to security-getGroupPerms.api --- .../core/security/SecurityApiActions.java | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 166bd857b6c..3bdece7a6bc 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -106,17 +106,17 @@ public class SecurityApiActions { public static class GetGroupPermsForm { - private boolean _includeEmptyPerms = true; + private boolean _includeEmptyPermGroups = true; private boolean _includeSubfolders = false; - public boolean isIncludeEmptyPerms() + public boolean isIncludeEmptyPermGroups() { - return _includeEmptyPerms; + return _includeEmptyPermGroups; } - public void setIncludeEmptyPerms(boolean includeEmptyPerms) + public void setIncludeEmptyPermGroups(boolean includeEmptyPermGroups) { - _includeEmptyPerms = includeEmptyPerms; + _includeEmptyPermGroups = includeEmptyPermGroups; } public boolean isIncludeSubfolders() @@ -134,50 +134,50 @@ public void setIncludeSubfolders(boolean includeSubfolders) public static class GetGroupPermsAction extends ReadOnlyApiAction { @Override - public ApiResponse execute(GetGroupPermsForm form, BindException errors) throws IOException + public ApiResponse execute(GetGroupPermsForm form, BindException errors) { - // Issue 53243: stream response from security-getGroupPerms.api Container container = getContainer(); - ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse()); - writer.startResponse(); - - writer.startObject("container"); - writeContainer(writer, container, SecurityManager.getGroups(container.getProject(), true), form.isIncludeSubfolders(), form.isIncludeEmptyPerms()); - writer.endObject(); + ApiSimpleResponse response = new ApiSimpleResponse(); - writer.endResponse(); + //if the container is not the root container, get the set of groups + //from the container's project and pass that down the recursion stack + response.put("container", getContainerPerms(container, + SecurityManager.getGroups(container.getProject(), true), + form.isIncludeSubfolders(), form.isIncludeEmptyPermGroups())); - return null; + return response; } - private void writeContainer(ApiJsonWriter writer, Container container, List groups, boolean includeSubfolders, boolean includeEmptyPerms) throws IOException + protected Map getContainerPerms(Container container, List groups, boolean includeSubfolders, boolean includeEmptyPermGroups) { - writer.writeProperty("path", container.getPath()); - writer.writeProperty("id", container.getId()); - writer.writeProperty("name", container.getName()); - writer.writeProperty("isInheritingPerms", container.isInheritedAcl()); - writer.writeProperty("groups", getGroupPerms(container, groups, includeEmptyPerms)); + Map containerPerms = new HashMap<>(); + containerPerms.put("path", container.getPath()); + containerPerms.put("id", container.getId()); + containerPerms.put("name", container.getName()); + containerPerms.put("isInheritingPerms", container.isInheritedAcl()); + containerPerms.put("groups", getGroupPerms(container, groups, includeEmptyPermGroups)); if (includeSubfolders && container.hasChildren()) { - writer.startList("children"); - + List> childPerms = new ArrayList<>(); for (Container child : container.getChildren()) { if (child.hasPermission(getUser(), ReadPermission.class)) { - writer.startObject(); - writeContainer(writer, child, child.isProject() ? SecurityManager.getGroups(child, true) : groups, includeSubfolders, includeEmptyPerms); - writer.endObject(); + childPerms.add(getContainerPerms(child, + child.isProject() ? SecurityManager.getGroups(child, true) : groups, + includeSubfolders, includeEmptyPermGroups)); } } - writer.endList(); + containerPerms.put("children", childPerms); } + + return containerPerms; } - protected List> getGroupPerms(Container container, List groups, boolean includeEmptyPerms) + protected List> getGroupPerms(Container container, List groups, boolean includeEmptyPermGroups) { List> groupsPerms = new ArrayList<>(); boolean isAdmin = container.hasPermission(getUser(), AdminPermission.class); @@ -185,7 +185,7 @@ protected List> getGroupPerms(Container container, List effectivePermissions = SecurityManager.getPermissionNames(container, group); - if (effectivePermissions.isEmpty() && !includeEmptyPerms) + if (effectivePermissions.isEmpty() && !includeEmptyPermGroups) continue; Map groupPerms = new HashMap<>(); @@ -194,24 +194,26 @@ protected List> getGroupPerms(Container container, List roles = SecurityManager.getEffectiveRoles(container, group).map(Role::getUniqueName).toList(); - groupPerms.put("roles", roles); + //add effective roles + List effectiveRoleList = SecurityManager.getEffectiveRoles(container, group).map(Role::getUniqueName).toList(); + groupPerms.put("roles", effectiveRoleList); groupPerms.put("effectivePermissions", effectivePermissions); if (isAdmin) { - List> parentGroups = new ArrayList<>(); + List> parentGroupInfos = new ArrayList<>(); group.getGroups().stream().forEach(parentGroupId -> { Group parentGroup = SecurityManager.getGroup(parentGroupId); if (parentGroup != null && parentGroup.getUserId() != group.getUserId()) { - parentGroups.add(getGroupMap(parentGroup)); + parentGroupInfos.add(getGroupMap(parentGroup)); } }); - groupPerms.put("groups", parentGroups); + groupPerms.put("groups", parentGroupInfos); } + + groupsPerms.add(groupPerms); } return groupsPerms; From ce77af9c3c7ac9c032e126163753aa2c3afcf197 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 16 Jul 2025 16:18:33 -0700 Subject: [PATCH 5/7] Remove unused imports --- core/src/org/labkey/core/security/SecurityApiActions.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 3bdece7a6bc..71a320b6666 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -19,7 +19,6 @@ import org.apache.commons.lang3.StringUtils; import org.json.JSONObject; import org.junit.Test; -import org.labkey.api.action.ApiJsonWriter; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; import org.labkey.api.action.ApiVersion; @@ -86,7 +85,6 @@ import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Date; From e0a5f7d59ef0430c292412fb8bdb21ca4f516702 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 16 Jul 2025 16:28:15 -0700 Subject: [PATCH 6/7] Undo --- api/src/org/labkey/api/action/ApiJsonWriter.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/api/src/org/labkey/api/action/ApiJsonWriter.java b/api/src/org/labkey/api/action/ApiJsonWriter.java index c284f71a068..f98a4e23ade 100644 --- a/api/src/org/labkey/api/action/ApiJsonWriter.java +++ b/api/src/org/labkey/api/action/ApiJsonWriter.java @@ -295,11 +295,6 @@ public void startObject(String name) throws IOException jg.writeObjectFieldStart(name); } - public void startObject() throws IOException - { - jg.writeStartObject(); - } - public void endObject() throws IOException { jg.writeEndObject(); From e38341865194b642ac26da087717e969e384e0d6 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Thu, 17 Jul 2025 09:26:35 -0700 Subject: [PATCH 7/7] @NotNull --- core/src/org/labkey/core/security/SecurityApiActions.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/security/SecurityApiActions.java b/core/src/org/labkey/core/security/SecurityApiActions.java index 71a320b6666..6c52e69f4f1 100644 --- a/core/src/org/labkey/core/security/SecurityApiActions.java +++ b/core/src/org/labkey/core/security/SecurityApiActions.java @@ -17,6 +17,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; import org.json.JSONObject; import org.junit.Test; import org.labkey.api.action.ApiResponse; @@ -147,7 +148,7 @@ public ApiResponse execute(GetGroupPermsForm form, BindException errors) return response; } - protected Map getContainerPerms(Container container, List groups, boolean includeSubfolders, boolean includeEmptyPermGroups) + protected Map getContainerPerms(Container container, @NotNull List groups, boolean includeSubfolders, boolean includeEmptyPermGroups) { Map containerPerms = new HashMap<>(); containerPerms.put("path", container.getPath()); @@ -175,7 +176,7 @@ protected Map getContainerPerms(Container container, List return containerPerms; } - protected List> getGroupPerms(Container container, List groups, boolean includeEmptyPermGroups) + protected List> getGroupPerms(Container container, @NotNull List groups, boolean includeEmptyPermGroups) { List> groupsPerms = new ArrayList<>(); boolean isAdmin = container.hasPermission(getUser(), AdminPermission.class);