Skip to content

Commit a810fb8

Browse files
labkey-adamlabkey-adam
authored
Give Troubleshooters read permission in the root (#7337)
1 parent 75a9861 commit a810fb8

30 files changed

Lines changed: 301 additions & 272 deletions

‎api/src/org/labkey/api/reports/report/view/ReportUtil.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,10 +738,10 @@ public static void updateReportSecurityPolicy(ViewContext context, @NotNull Repo
738738
{
739739
MutableSecurityPolicy policy = new MutableSecurityPolicy(report.getDescriptor(), SecurityPolicyManager.getPolicy(report.getDescriptor(), false));
740740

741-
List<Role> principalAssignedRoles = policy.getAssignedRoles(principal);
742-
if (toAdd && principalAssignedRoles.isEmpty())
741+
boolean hasNoAssignedRoles = policy.getAssignedRoles(principal).findAny().isEmpty();
742+
if (toAdd && hasNoAssignedRoles)
743743
policy.addRoleAssignment(principal, ReaderRole.class);
744-
else if (!toAdd && !principalAssignedRoles.isEmpty())
744+
else if (!toAdd && !hasNoAssignedRoles)
745745
policy.addRoleAssignment(principal, NoPermissionsRole.class);
746746

747747
SecurityPolicyManager.savePolicy(policy, context.getUser());

‎api/src/org/labkey/api/security/Group.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import org.labkey.api.data.Container;
1919
import org.labkey.api.data.ContainerManager;
20+
import org.labkey.api.data.Transient;
2021
import org.labkey.api.security.roles.Role;
2122

2223
import java.util.stream.Stream;
@@ -95,6 +96,7 @@ public String getPath()
9596
return "/" + c.getName() + "/" + getName();
9697
}
9798

99+
@Transient
98100
@Override
99101
public PrincipalArray getGroups()
100102
{
@@ -111,7 +113,7 @@ public boolean isInGroup(int group)
111113
public Stream<Role> getAssignedRoles(SecurableResource resource)
112114
{
113115
SecurityPolicy policy = SecurityPolicyManager.getPolicy(resource);
114-
return policy.getRoles(getGroups()).stream();
116+
return policy.getRoles(getGroups());
115117
}
116118

117119
@Override

‎api/src/org/labkey/api/security/LimitedUser.java‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.labkey.api.util.TestContext;
4242

4343
import java.util.Date;
44+
import java.util.Objects;
4445
import java.util.Set;
4546
import java.util.stream.Collectors;
4647
import java.util.stream.Stream;
@@ -77,7 +78,7 @@ public PrincipalArray getGroups(User user)
7778
@Override
7879
public Stream<Role> getAssignedRoles(User user, SecurableResource resource)
7980
{
80-
return _roles.stream();
81+
return Objects.requireNonNull(_roles).stream();
8182
}
8283
}
8384

@@ -111,38 +112,39 @@ public void testLimitedUser()
111112
{
112113
User user = TestContext.get().getUser();
113114

114-
testPermissions(new LimitedUser(user), 0, false, false, false, false, false);
115-
testPermissions(new LimitedUser(user, ReaderRole.class), 1, true, false, false, false, false);
116-
testPermissions(new LimitedUser(user, EditorRole.class), 1, true, true, true, false, false);
117-
testPermissions(new LimitedUser(user, FolderAdminRole.class), 1, true, true, true, true, true);
118-
testPermissions(new LimitedUser(new LimitedUser(user, FolderAdminRole.class), ReaderRole.class), 1, true, false, false, false, false);
115+
testPermissions(new LimitedUser(user), 0, 0, false, false, false, false, false);
116+
testPermissions(new LimitedUser(user, ReaderRole.class), 1, 0, true, false, false, false, false);
117+
testPermissions(new LimitedUser(user, EditorRole.class), 1, 0, true, true, true, false, false);
118+
testPermissions(new LimitedUser(user, FolderAdminRole.class), 1, 0, true, true, true, true, true);
119+
testPermissions(new LimitedUser(new LimitedUser(user, FolderAdminRole.class), ReaderRole.class), 1, 0, true, false, false, false, false);
119120
}
120121

121122
@Test
122123
public void testElevatedUser()
123124
{
124125
User user = TestContext.get().getUser();
125126
Container c = JunitUtil.getTestContainer();
127+
Container root = ContainerManager.getRoot();
126128

127-
testPermissions(ElevatedUser.getElevatedUser(new LimitedUser(user, SubmitterRole.class, null), ReaderRole.class, null), 2, true, true, false, false, false);
128-
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, new LimitedUser(user)), 1, false, false, false, false, true);
129-
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, new LimitedUser(user, ReaderRole.class)), 2, true, false, false, false, true);
130-
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, ElevatedUser.getElevatedUser(new LimitedUser(user, ReaderRole.class), EditorRole.class)), 3, true, true, true, false, true);
129+
testPermissions(ElevatedUser.getElevatedUser(new LimitedUser(user, SubmitterRole.class, null), ReaderRole.class, null), 2, 0, true, true, false, false, false);
130+
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, new LimitedUser(user)), 1, 1, false, false, false, false, true);
131+
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, new LimitedUser(user, ReaderRole.class)), 2, 1, true, false, false, false, true);
132+
testPermissions(ElevatedUser.ensureCanSeeAuditLogRole(c, ElevatedUser.getElevatedUser(new LimitedUser(user, ReaderRole.class), EditorRole.class)), 3, 1, true, true, true, false, true);
131133

132134
int groupCount = user.getGroups().size();
133135
int roleCount = (int)user.getAssignedRoles(c).count();
134-
int siteRolesCount = (int)user.getSiteRoles().count();
136+
int siteRolesCount = (int)user.getSiteRoles(root).count();
135137
User elevated = ElevatedUser.getElevatedUser(user);
136138
assertEquals(groupCount, elevated.getGroups().size());
137139
assertEquals(roleCount, (int)elevated.getAssignedRoles(c).count());
138-
assertEquals(siteRolesCount, (int)elevated.getSiteRoles().count());
140+
assertEquals(siteRolesCount, (int)elevated.getSiteRoles(root).count());
139141
}
140142

141-
private void testPermissions(User user, int roleCount, boolean hasRead, boolean hasInsert, boolean hasUpdate, boolean hasAdmin, boolean hasCanSeeAuditLog)
143+
private void testPermissions(User user, int roleCount, int siteRoleCount, boolean hasRead, boolean hasInsert, boolean hasUpdate, boolean hasAdmin, boolean hasCanSeeAuditLog)
142144
{
143145
Container c = JunitUtil.getTestContainer();
144146
assertEquals(roleCount, (int)user.getAssignedRoles(c).count());
145-
assertTrue(user.getSiteRoles().findAny().isEmpty());
147+
assertEquals(siteRoleCount, user.getSiteRoles(ContainerManager.getRoot()).count());
146148
assertFalse(user.hasSiteAdminPermission());
147149
assertEquals(0, user.getGroups().stream().count());
148150
assertFalse(user.hasPrivilegedRole());

‎api/src/org/labkey/api/security/RoleSet.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.junit.Assert;
2121
import org.junit.Test;
2222
import org.labkey.api.data.Container;
23+
import org.labkey.api.data.ContainerManager;
2324
import org.labkey.api.pipeline.PipelineJob;
2425
import org.labkey.api.security.impersonation.RoleImpersonationContextFactory;
2526
import org.labkey.api.security.roles.CanSeeAuditLogRole;
@@ -153,15 +154,15 @@ private void testImpersonateRoles(User adminUser, @Nullable Container project, C
153154
impersonatingUser.setImpersonationContext(factory.getImpersonationContext());
154155

155156
if (null == project)
156-
assertEquals(roles, impersonatingUser.getSiteRoles().collect(Collectors.toSet()));
157+
assertEquals(roles, impersonatingUser.getSiteRoles(ContainerManager.getRoot()).collect(Collectors.toSet()));
157158
else
158159
assertEquals(roles, impersonatingUser.getAssignedRoles(project).collect(Collectors.toSet()));
159160

160161
json = writer.writeValueAsString(impersonatingUser);
161162
User reconstitutedUser = mapper.readValue(json, User.class);
162163

163164
if (null == project)
164-
assertEquals(roles, reconstitutedUser.getSiteRoles().collect(Collectors.toSet()));
165+
assertEquals(roles, reconstitutedUser.getSiteRoles(ContainerManager.getRoot()).collect(Collectors.toSet()));
165166
else
166167
assertEquals(roles, reconstitutedUser.getAssignedRoles(project).collect(Collectors.toSet()));
167168
}

‎api/src/org/labkey/api/security/SecurityManager.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,7 +1950,7 @@ public static Collection<Integer> getFolderUserids(Container c)
19501950

19511951
//don't filter if all site users is playing a role
19521952
Group allSiteUsers = getGroup(Group.groupUsers);
1953-
if (!policy.getAssignedRoles(allSiteUsers).isEmpty())
1953+
if (policy.getAssignedRoles(allSiteUsers).findAny().isPresent())
19541954
{
19551955
// Just select all users
19561956
SQLFragment sql = new SQLFragment("SELECT u.UserId FROM ");
@@ -3122,6 +3122,8 @@ public static List<String> getPermissionNames(SecurableResource resource, @NotNu
31223122
* appropriate only for generating reports about role assignments for administrators.
31233123
* Returns the roles the principal is playing in this securable resource, either due to direct assignment or due
31243124
* to membership in a group that is assigned the role.
3125+
* Note: The returned stream may duplicate some roles; if a distinct stream of roles is required, callers should
3126+
* invoke {@code distinct()} or collect to a set.
31253127
* @param principal The principal
31263128
* @return The roles this principal is playing in the securable resource
31273129
*/
@@ -3376,7 +3378,7 @@ public PrincipalArray getGroups()
33763378
public Stream<Role> getAssignedRoles(SecurableResource resource)
33773379
{
33783380
SecurityPolicy policy = SecurityPolicyManager.getPolicy(resource);
3379-
return policy.getRoles(getGroups()).stream();
3381+
return policy.getRoles(getGroups());
33803382
}
33813383

33823384
@Override
@@ -3560,9 +3562,13 @@ public boolean performChecks()
35603562
// check that the user has the expected role
35613563
Container rootContainer = ContainerManager.getRoot();
35623564
User user = UserManager.getUser(userEmail);
3563-
Collection<Role> roles = rootContainer.getPolicy().getAssignedRoles(user);
3565+
assertNotNull(user);
35643566
Role role = RoleManager.getRole(TEST_USER_1_ROLE_NAME);
3565-
assertTrue("The user defined in the startup properties: " + userEmail + " did not have the specified role: " + TEST_USER_1_ROLE_NAME, roles.contains(role));
3567+
assertTrue(
3568+
"The user defined in the startup properties: " + userEmail + " did not have the specified role: " + TEST_USER_1_ROLE_NAME,
3569+
rootContainer.getPolicy().getAssignedRoles(user)
3570+
.anyMatch(r -> r.equals(role))
3571+
);
35663572

35673573
// delete the test user that was added
35683574
UserManager.deleteUser(user.getUserId());
@@ -3598,9 +3604,14 @@ public boolean performChecks()
35983604

35993605
// check that the group has the expected role
36003606
Group group = GroupManager.getGroup(rootContainer, TEST_GROUP_1_NAME, GroupEnumType.SITE);
3601-
Collection<Role> roles = rootContainer.getPolicy().getAssignedRoles(group);
3607+
assertNotNull(group);
36023608
Role role = RoleManager.getRole(TEST_GROUP_1_ROLE_NAME);
3603-
assertTrue("The group defined in the startup properties: " + TEST_GROUP_1_NAME + " did not have the specified role: " + TEST_GROUP_1_ROLE_NAME, roles.contains(role));
3609+
assertNotNull(role);
3610+
assertTrue(
3611+
"The group defined in the startup properties: " + TEST_GROUP_1_NAME + " did not have the specified role: " + TEST_GROUP_1_ROLE_NAME,
3612+
rootContainer.getPolicy().getAssignedRoles(group)
3613+
.anyMatch(r -> r.equals(role))
3614+
);
36043615

36053616
// delete the test group that was added
36063617
deleteGroup(group, TestContext.get().getUser());

0 commit comments

Comments
 (0)