Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue 48499: Use preferred SecurityPolicyManager.savePolicy() variant #375

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Issue 48499: Use preferred SecurityPolicyManager.savePolicy() variant #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94bcf23
Issue 48499: Use preferred SecurityPolicyManager.savePolicy() variant
labkey-jeckels Aug 26, 2023
908769f
User may not have direct access
labkey-jeckels Aug 28, 2023
639993f
Another try at the right mix of users
labkey-jeckels Aug 28, 2023
85bedee
In callers of SecurityPolicyManager.savePolicy() in CopyExperimentFin…
vagisha Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions SkylineToolsStore/src/org/labkey/skylinetoolsstore/SkylineToolsStoreController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ protected Container makeContainer(Container parent, String folderName, List<User
for (User u : users)
policy.addRoleAssignment(u, role);

SecurityPolicyManager.savePolicy(policy);
SecurityPolicyManager.savePolicy(policy, User.getAdminServiceUser());

return c;
}
Expand Down Expand Up @@ -319,7 +319,7 @@ protected void copyContainerPermissions(Container from, Container to)
if (from == null || to == null)
return;

SecurityPolicyManager.savePolicy(copyPolicy(to, from.getPolicy()));
SecurityPolicyManager.savePolicy(copyPolicy(to, from.getPolicy()), User.getAdminServiceUser());
}

public static SkylineTool[] sortToolsByCreateDate(SkylineTool[] tools)
Expand Down Expand Up @@ -1359,7 +1359,7 @@ public ModelAndView handleRequestInternal(HttpServletRequest httpServletRequest,
for (User u : newToolEditors)
policy.addRoleAssignment(u, RoleManager.getRole(EditorRole.class));
policy = filterPolicy(policy, toolOwnersUsers, new Role[]{RoleManager.getRole(EditorRole.class), RoleManager.getRole(FolderAdminRole.class)});
SecurityPolicyManager.savePolicy(policy);
SecurityPolicyManager.savePolicy(policy, User.getAdminServiceUser());

Container toolStoreContainer = tool != null ? tool.getContainerParent() : getContainer();

Expand Down
4 changes: 2 additions & 2 deletions panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ else if(error.length() > 0)
// Assign project admin role to the group.
MutableSecurityPolicy policy = new MutableSecurityPolicy(SecurityPolicyManager.getPolicy(container));
policy.addRoleAssignment(group, ProjectAdminRole.class);
SecurityPolicyManager.savePolicy(policy);
SecurityPolicyManager.savePolicy(policy, getUser());

// Add the journal
_journal = new Journal();
Expand Down Expand Up @@ -6599,7 +6599,7 @@ private void makeFolderPublic(JournalManager.PublicDataUser publicDataUser)
{
newPolicy.addRoleAssignment(publicDataUser.getUser(), ReaderRole.class);
}
SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, User.getAdminServiceUser());
}

private void addDownloadDataWebpart(Container container)
Expand Down
2 changes: 1 addition & 1 deletion panoramapublic/src/org/labkey/panoramapublic/pipeline/AssignSubmitterPermissionJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private boolean assignRole(User user, String userType, Container container, bool
logger.info(String.format("'%s', %s: %s - %s", container.getPath(), userType, user.getEmail(), "assigning"));
MutableSecurityPolicy newPolicy = new MutableSecurityPolicy(container, container.getPolicy());
newPolicy.addRoleAssignment(user, PanoramaPublicSubmitterRole.class, false);
SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, User.getAdminServiceUser());
return true;
}
else
Expand Down
10 changes: 5 additions & 5 deletions panoramapublic/src/org/labkey/panoramapublic/pipeline/CopyExperimentFinalTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ private Pair<User, String> assignReviewer(JournalSubmission js, ExperimentAnnota
{
throw new PipelineJobException("Error creating a new account for reviewer", e);
}
assignReader(reviewer, targetExperiment.getContainer());
assignReader(reviewer, targetExperiment.getContainer(), user);
js.getJournalExperiment().setReviewer(reviewer.getUserId());
SubmissionManager.updateJournalExperiment(js.getJournalExperiment(), user);

Expand All @@ -370,7 +370,7 @@ private Pair<User, String> assignReviewer(JournalSubmission js, ExperimentAnnota
{
// Assign Site:Guests to reader role
log.info("Making folder public.");
assignReader(SecurityManager.getGroup(Group.groupGuests), targetExperiment.getContainer());
assignReader(SecurityManager.getGroup(Group.groupGuests), targetExperiment.getContainer(), user);
}
return new Pair<>(null, null);
}
Expand Down Expand Up @@ -544,7 +544,7 @@ private void updatePermissions(User formSubmitter, ExperimentAnnotations targetE
assignPanoramaPublicSubmitterRole(newPolicy, log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(),
formSubmitter); // User that submitted the form. Can be different from the user selected as the data submitter

SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, pipelineJobUser);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be OK to pass in the User that started the pipeline job instead of User.getAdminServiceUser(). I will revert the changes in my commit if tests fail due to this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


addToSubmittersGroup(target.getProject(), log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(), formSubmitter);
}
Expand Down Expand Up @@ -650,11 +650,11 @@ private User createReviewerAccount(String reviewerEmailPrefix, String password,
return newUser.getUser();
}

private void assignReader(UserPrincipal reader, Container target)
private void assignReader(UserPrincipal reader, Container target, User pipelineJobUser)
{
MutableSecurityPolicy newPolicy = new MutableSecurityPolicy(target, target.getPolicy());
newPolicy.addRoleAssignment(reader, ReaderRole.class);
SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, pipelineJobUser);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. It should be OK to pass in the User that started the pipeline job instead of User.getAdminServiceUser().

}

public static String createPassword()
Expand Down
8 changes: 4 additions & 4 deletions panoramapublic/src/org/labkey/panoramapublic/query/JournalManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private static void addPermission(Container folder, UserPrincipal journalGroup)
}
}
newPolicy.addRoleAssignment(journalGroup, CopyTargetedMSExperimentRole.class, false);
SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, User.getAdminServiceUser());
}

private static void removePermission(Container folder, UserPrincipal journalGroup)
Expand All @@ -260,7 +260,7 @@ private static void removePermission(Container folder, UserPrincipal journalGrou
newPolicy.addRoleAssignment(journalGroup, role);
}
}
SecurityPolicyManager.savePolicy(newPolicy);
SecurityPolicyManager.savePolicy(newPolicy, User.getAdminServiceUser());
}

public static void addJournalPermissions(ExperimentAnnotations exptAnnotations, UserPrincipal journalGroup, User user)
Expand Down Expand Up @@ -293,13 +293,13 @@ public static ShortURLRecord saveShortURL(ActionURL longURL, String shortUrl, @N
throw new ValidationException("Error saving link \"" + shortUrl + "\". It may already be in use. Error message was: " + e.getMessage());
}

if(journalGroup != null)
if (journalGroup != null)
{
MutableSecurityPolicy policy = new MutableSecurityPolicy(SecurityPolicyManager.getPolicy(shortAccessURLRecord));
// Add a role assignment to let another group manage the URL. This grants permission to the journal
// to change where the URL redirects you to after they copy the data
policy.addRoleAssignment(journalGroup, EditorRole.class);
SecurityPolicyManager.savePolicy(policy);
SecurityPolicyManager.savePolicy(policy, User.getAdminServiceUser());
}
return shortAccessURLRecord;
}
Expand Down
2 changes: 1 addition & 1 deletion panoramapublic/src/org/labkey/panoramapublic/query/SubmissionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ private static void ensureEditorRole(@NotNull ShortURLRecord shortUrl, User user
if (!isEditor)
{
policy.addRoleAssignment(user, EditorRole.class);
SecurityPolicyManager.savePolicy(policy);
SecurityPolicyManager.savePolicy(policy, User.getAdminServiceUser());
}
}

Expand Down