Skip to content

task(content-drive): add permissions tab when editing folders #35160#35327

Open
zJaaal wants to merge 7 commits intomainfrom
35160-task-content-drive-add-permissions-tab-when-editing-folders
Open

task(content-drive): add permissions tab when editing folders #35160#35327
zJaaal wants to merge 7 commits intomainfrom
35160-task-content-drive-add-permissions-tab-when-editing-folders

Conversation

@zJaaal
Copy link
Copy Markdown
Member

@zJaaal zJaaal commented Apr 15, 2026

Summary

  • Added Edit Permissions option to the folder context menu in Content Drive, shown only when the user has EDIT_PERMISSIONS permission on the folder
  • Opens DotPermissionsIframeDialogComponent loading /html/portlet/ext/folders/permissions.jsp in an iframe
  • Added permissions.jsp to serve the existing permissions portlet in popup mode for folders
  • Changed permissions field in DotContentDriveFolder from number[] to string[] and added PERMISSIONS_TYPE const enum
  • Replaced page links with current page report (Page {currentPage}) in folder list view pagination

Test plan

  • Right-click a folder where you have EDIT_PERMISSIONS — verify Edit Permissions appears in the context menu
  • Right-click a folder where you lack EDIT_PERMISSIONS — verify the option is absent
  • Click Edit Permissions — verify the dialog opens with the permissions portlet loaded
  • Verify dialog closes with ESC key and X button
  • Run unit tests: yarn nx test dot-content-drive

🤖 Generated with Claude Code

This PR fixes: #35160

@zJaaal zJaaal linked an issue Apr 15, 2026 that may be closed by this pull request
5 tasks
@github-actions github-actions bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Apr 15, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @zJaaal's task in 2m 26s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze and post review

Overall the change is clean and follows existing patterns well. A few issues worth addressing:

1. Empty context menu is shown when folder has no permissions (UX/fragility)

dot-folder-list-context-menu.component.ts:168–174

this.$items.set(folderMenuItems);
this.$memoizedMenuItems.set({ ...this.$memoizedMenuItems(), [key]: folderMenuItems });
this.contextMenu()?.show(triggeredEvent);   // ← called even when folderMenuItems is []
return;

When a folder has zero permissions, contextMenu().show() is still called with an empty item list. The PR author's response was that PrimeNG closes the menu immediately, but this is relying on undocumented internal behavior — it's fragile and produces a visible flash. The fix is one line:

if (folderMenuItems.length > 0) {
    this.contextMenu()?.show(triggeredEvent);
}

The corresponding spec test (should build empty menu when folder has no permissions) also does not assert that show() is suppressed, so this path is under-verified. Fix this →


2. JSP has no server-side EDIT_PERMISSIONS check

dotCMS/src/main/webapp/html/portlet/ext/folders/permissions.jsp:8–18

The JSP retrieves and renders the permissions UI for any authenticated user who can call FolderAPI.find(). There is no check that the requesting user actually holds EDIT_PERMISSIONS on the folder before setting PERMISSIONABLE_EDIT and rendering the tab. Direct URL access bypasses the frontend menu-item guard entirely.

edit_permissions_tab_inc.jsp:44 does set doesUserHavePermissionsToEdit as a JS boolean that gates the save button, but that is client-side and trivially bypassed.

categories/permissions.jsp and contentlet/permissions.jsp carry the same gap, so this is consistent with the existing pattern — the actual write enforcement lives in the DWR PermissionAjax backend service. That said, adding an explicit server-side read guard here would be a defense-in-depth improvement and would prevent the permissions form from being rendered at all for unauthorized users:

if (folder != null && UtilMethods.isSet(folder.getInode())) {
    if (!APILocator.getPermissionAPI()
            .doesUserHavePermission(folder, PermissionAPI.PERMISSION_EDIT_PERMISSIONS, user)) {
        response.sendError(HttpServletResponse.SC_FORBIDDEN);
        return;
    }
    request.setAttribute(com.dotmarketing.util.WebKeys.PERMISSIONABLE_EDIT, folder);
    // ...
}

3. Stale memoized menu after permission change

dot-folder-list-context-menu.component.ts:99–105

The memoized menu is only cleared on DotContentDriveStatus.LOADING. If a user's permissions on a folder are changed by an admin while the page is open (without a full reload), the cached menu still reflects the old permission set — the "Edit Permissions" item may appear or disappear incorrectly until the next store reload. This is a pre-existing limitation of the memoization approach, but it is now materially more visible since permissions directly control which items appear. Low priority, but worth a // TODO comment or a tracked follow-up.


Minor: currentPageReportTemplate interpolation

dot-folder-list-view.component.html:28

[currentPageReportTemplate]="`${('Page' | dm)} {currentPage}`"

'Page' | dm is applied at Angular template evaluation time and is reactive to locale changes, which is correct. However, using a template literal here means Angular evaluates the expression on every change-detection cycle. Consider binding the translated string to a component property so it isn't recomputed on every tick — not a blocking issue, just avoidable overhead.

Comment thread core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts Outdated
zJaaal and others added 2 commits April 15, 2026 13:55
- Derive PermissionType union from PERMISSIONS_TYPE const and narrow
  DotContentDriveFolder.permissions from string[] to PermissionType[]
- Replace .find() with .includes() for permission checks in context menu
- Add test for folder with empty permissions yielding zero menu items

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zJaaal zJaaal marked this pull request as ready for review April 15, 2026 17:01
…ng-folders' of https://github.com/dotCMS/core into 35160-task-content-drive-add-permissions-tab-when-editing-folders
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Content Drive: add Permissions tab when editing folders

4 participants