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.

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

Issue 52254: Folder admins cannot choose a template folder #6315

Merged

Conversation

labkey-jeckels
Copy link
Contributor

@labkey-jeckels labkey-jeckels commented Feb 13, 2025

Rationale

Folder admins don't have access to the root container, so getContainers sends neither an effectivePermissions array nor a path.

The old behavior was to call LABKEY.Security.hasPermission() which did a bitwise AND, so therefore tolerated an undefined value

#5718

Changes

  • Check if we have permissions to review

@@ -622,7 +622,7 @@
const getTemplateFolders = function(data) {
// add the container itself to the templateFolder object if it is not the root and the user has admin perm to it
// and if it is not a workbook or container tab folder
if (data.path !== "/" && LABKEY.Security.hasEffectivePermission(data.effectivePermissions, LABKEY.Security.effectivePermissions.admin)
if (data.path !== "/" && data.effectivePermissions && LABKEY.Security.hasEffectivePermission(data.effectivePermissions, LABKEY.Security.effectivePermissions.admin)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could/should consider also making LABKEY.Security.hasEffectivePermission() defensive of falsey inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I briefly considered that and seems like it would be a fine approach too. Would that be here?

for (var i = 0; i < effectivePermissions.length; i++)

I'm confused about how the TS version is or isn't involved.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its OK to not do this in 24.11 and just put the change in as you have it.

Additionally, I've tried to describe how the client API is served. Let me know if this is helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, I've tried to describe how the client API is served. Let me know if this is helpful.

Yes, definitely. I hope to be able to find it in the future.

@labkey-jeckels labkey-jeckels merged commit 434c034 into release24.11-SNAPSHOT Feb 14, 2025
11 of 12 checks passed
@labkey-jeckels labkey-jeckels deleted the 24.11_fb_52254_folderAdminTemplateFolder branch February 14, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants