From 4cb1fac97efb385d3c44871448e574f1ac829d30 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Wed, 31 Jul 2024 20:14:48 -0700 Subject: [PATCH 1/3] Add SND role impersonation --- .../snd/security/SNDSecurityManager.java | 14 +++++++++ .../roles/SNDRoleImpersonationContext.java | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java diff --git a/src/org/labkey/snd/security/SNDSecurityManager.java b/src/org/labkey/snd/security/SNDSecurityManager.java index 86c5309e..19cb1dff 100644 --- a/src/org/labkey/snd/security/SNDSecurityManager.java +++ b/src/org/labkey/snd/security/SNDSecurityManager.java @@ -35,6 +35,8 @@ import org.labkey.api.security.SecurityPolicy; import org.labkey.api.security.SecurityPolicyManager; import org.labkey.api.security.User; +import org.labkey.api.security.impersonation.ImpersonationContext; +import org.labkey.api.security.impersonation.RoleImpersonationContextFactory; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.roles.Role; import org.labkey.api.snd.Category; @@ -44,6 +46,7 @@ import org.labkey.api.snd.SuperPackage; import org.labkey.api.view.NotFoundException; import org.labkey.snd.SNDManager; +import org.labkey.snd.security.roles.SNDRoleImpersonationContext; import java.util.ArrayList; import java.util.Collections; @@ -206,6 +209,17 @@ public Map getAllSecurityRoles() private boolean hasPermission(User u, Category category, QCStateActionEnum action, QCStateEnum qcState) { Permission perm = action.getPermission(qcState); + + // SND has permissions bound to categories which can be assigned to packages (domains). Impersonating roles is used + // in automated and manual testing to verify this behavior. The behavior of role impersonation was changed in core + // labkey to only check for roles related to containers. This is a workaround to go back to checking all roles. + ImpersonationContext impersonationContext = u.getImpersonationContext(); + if (impersonationContext instanceof RoleImpersonationContextFactory.RoleImpersonationContext context) + { + ImpersonationContext sndContext = new SNDRoleImpersonationContext(context.getImpersonationProject(), context.getAdminUser(), context.getRoles(), context.getFactory(), context.getCacheKey()); + u.setImpersonationContext(sndContext); + } + return perm != null && category.hasPermission(u, perm.getClass()); } diff --git a/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java b/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java new file mode 100644 index 00000000..41c0a4a1 --- /dev/null +++ b/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java @@ -0,0 +1,29 @@ +package org.labkey.snd.security.roles; + +import org.jetbrains.annotations.Nullable; +import org.labkey.api.data.Container; +import org.labkey.api.security.RoleSet; +import org.labkey.api.security.SecurableResource; +import org.labkey.api.security.User; +import org.labkey.api.security.impersonation.ImpersonationContextFactory; +import org.labkey.api.security.impersonation.RoleImpersonationContextFactory; +import org.labkey.api.security.roles.Role; + +import java.util.HashSet; +import java.util.Objects; +import java.util.stream.Stream; + +public class SNDRoleImpersonationContext extends RoleImpersonationContextFactory.RoleImpersonationContext +{ + public SNDRoleImpersonationContext(@Nullable Container project, User adminUser, RoleSet roles, ImpersonationContextFactory factory, String cacheKey) + { + super(project, adminUser, roles, factory, cacheKey); + } + + @Override + public Stream getAssignedRoles(User user, SecurableResource resource) + { + RoleSet _roles = Objects.requireNonNullElse(getRoles(), new RoleSet(new HashSet<>())); + return _roles.stream(); + } +} From 403752c033fdd4f475053f103ba9f144201b4c0d Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Thu, 1 Aug 2024 04:35:03 -0700 Subject: [PATCH 2/3] Fix test response --- webapp/snd/test/driver.js | 3 +++ webapp/snd/test/tests.js | 14 +++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/webapp/snd/test/driver.js b/webapp/snd/test/driver.js index 1994e95d..44a8720a 100644 --- a/webapp/snd/test/driver.js +++ b/webapp/snd/test/driver.js @@ -1113,6 +1113,9 @@ else if (LABKEY.Utils.isObject(result)) { finish(undefined, result); } + else if (json?.event?.exception?.message) { + finish('Test "' + test.name + '" failed. ' + json.event.exception.message); + } else { finish('Test "' + test.name + '" failed. Test should specify a reason for failure or return true.'); } diff --git a/webapp/snd/test/tests.js b/webapp/snd/test/tests.js index e5fa1ecf..f842e040 100644 --- a/webapp/snd/test/tests.js +++ b/webapp/snd/test/tests.js @@ -348,7 +348,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -563,7 +563,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -586,7 +586,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -630,7 +630,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -655,7 +655,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -713,7 +713,7 @@ return true; } - LABKEY.handleFailure(response, name + " - Stack Trace"); + LABKEY.handleSndFailure(response, name + " - Stack Trace"); return false; } } @@ -782,7 +782,7 @@ return true; } - LABKEY.handleFailure(response, test + " - Stack Trace"); + LABKEY.handleSndFailure(response, test + " - Stack Trace"); return false; } }; From a538d148722fbe4beffcb764762bbde15d8a12d0 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Mon, 5 Aug 2024 08:38:07 -0700 Subject: [PATCH 3/3] CR refactor --- .../snd/security/SNDSecurityManager.java | 15 ++++++---- .../roles/SNDRoleImpersonationContext.java | 29 ------------------- 2 files changed, 10 insertions(+), 34 deletions(-) delete mode 100644 src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java diff --git a/src/org/labkey/snd/security/SNDSecurityManager.java b/src/org/labkey/snd/security/SNDSecurityManager.java index 19cb1dff..41583c0c 100644 --- a/src/org/labkey/snd/security/SNDSecurityManager.java +++ b/src/org/labkey/snd/security/SNDSecurityManager.java @@ -46,7 +46,6 @@ import org.labkey.api.snd.SuperPackage; import org.labkey.api.view.NotFoundException; import org.labkey.snd.SNDManager; -import org.labkey.snd.security.roles.SNDRoleImpersonationContext; import java.util.ArrayList; import java.util.Collections; @@ -209,18 +208,24 @@ public Map getAllSecurityRoles() private boolean hasPermission(User u, Category category, QCStateActionEnum action, QCStateEnum qcState) { Permission perm = action.getPermission(qcState); + if (perm == null) + { + return false; + } + + Set roles = Set.of(); - // SND has permissions bound to categories which can be assigned to packages (domains). Impersonating roles is used + // SND has permissions bound to SND categories which can be assigned to packages (domains). Impersonating roles is used // in automated and manual testing to verify this behavior. The behavior of role impersonation was changed in core // labkey to only check for roles related to containers. This is a workaround to go back to checking all roles. ImpersonationContext impersonationContext = u.getImpersonationContext(); if (impersonationContext instanceof RoleImpersonationContextFactory.RoleImpersonationContext context) { - ImpersonationContext sndContext = new SNDRoleImpersonationContext(context.getImpersonationProject(), context.getAdminUser(), context.getRoles(), context.getFactory(), context.getCacheKey()); - u.setImpersonationContext(sndContext); + roles = context.getRoles().getRoles(); } - return perm != null && category.hasPermission(u, perm.getClass()); + return SecurityManager.hasAllPermissions(this.getClass().getName() + ":" + category.getResourceName(), + category, u, Set.of(perm.getClass()), roles); } diff --git a/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java b/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java deleted file mode 100644 index 41c0a4a1..00000000 --- a/src/org/labkey/snd/security/roles/SNDRoleImpersonationContext.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.labkey.snd.security.roles; - -import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.Container; -import org.labkey.api.security.RoleSet; -import org.labkey.api.security.SecurableResource; -import org.labkey.api.security.User; -import org.labkey.api.security.impersonation.ImpersonationContextFactory; -import org.labkey.api.security.impersonation.RoleImpersonationContextFactory; -import org.labkey.api.security.roles.Role; - -import java.util.HashSet; -import java.util.Objects; -import java.util.stream.Stream; - -public class SNDRoleImpersonationContext extends RoleImpersonationContextFactory.RoleImpersonationContext -{ - public SNDRoleImpersonationContext(@Nullable Container project, User adminUser, RoleSet roles, ImpersonationContextFactory factory, String cacheKey) - { - super(project, adminUser, roles, factory, cacheKey); - } - - @Override - public Stream getAssignedRoles(User user, SecurableResource resource) - { - RoleSet _roles = Objects.requireNonNullElse(getRoles(), new RoleSet(new HashSet<>())); - return _roles.stream(); - } -}