Skip to content

fix: skip VIEW Rules permission check when page has no rules (#35292)#35404

Draft
gortiz-dotcms wants to merge 3 commits intomainfrom
issue-35292-copy-rules-permission
Draft

fix: skip VIEW Rules permission check when page has no rules (#35292)#35404
gortiz-dotcms wants to merge 3 commits intomainfrom
issue-35292-copy-rules-permission

Conversation

@gortiz-dotcms
Copy link
Copy Markdown
Member

@gortiz-dotcms gortiz-dotcms commented Apr 21, 2026

Summary

Fixes #35292

When a non-admin user ran the Copy workflow action on an HTML page, it failed with:

Unable to execute the Workflow Action. Error: User does not have permissions to VIEW Rules

This happened even when the page had no rules attached.

Root Cause

RulesAPIImpl.getAllRulesByParent() enforced the VIEW Rules permission check before querying whether any rules existed for the page. As a result, any user without explicit View Rules permission on the site/folder was blocked — even when there was nothing to view or copy.

Fix

Moved the permission check after the rules query. If the page has no rules, the method returns an empty list immediately without enforcing the permission. The check is only reached when there are actual rules to expose.

// Before
checkRulePermission(user, contentletParent, PermissionAPI.PERMISSION_READ, respectFrontendRoles);
return rulesFactory.getAllRulesByParent(parent);

// After
final List<Rule> rules = rulesFactory.getAllRulesByParent(parent);
if (rules.isEmpty()) {
    return Collections.emptyList();
}
checkRulePermission(user, contentletParent, PermissionAPI.PERMISSION_READ, respectFrontendRoles);
return rules;

This restores the v5.x behavior where users with edit/publish permissions on a page could copy it freely when no rules were involved.

Impact

  • Users without View Rules permission can now copy HTML pages that have no rules attached.
  • Users with rules on their pages still require View Rules permission — no change in behavior there.
  • No schema, API contract, or cache changes.

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

Claude finished @gortiz-dotcms's task in 3m 39s —— View job


🔍 dotCMS Backend Review

  • Load dotCMS conventions
  • Get Java diff
  • Run specialized sub-agents (Security, Database, Java Standards, REST API)
  • Consolidate findings and post review

Review posted above. The contentletParent DB-lookup placement issue from the previous review is resolved. Two medium findings remain (authorization oracle and write-operation authorization contract). Java standards, database, and REST API sub-agents found no issues.

@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Apr 21, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

🔍 dotCMS Backend Review

The contentletParent DB-lookup placement issue flagged in the previous review has been correctly resolved — it now sits after the rules.isEmpty() guard.

Two medium findings remain.


[🟡 Medium] dotCMS/src/enterprise/java/com/dotcms/enterprise/rules/RulesAPIImpl.java:233-236

Skipping the permission check when no rules exist creates an authorization oracle. An unauthorized user receives two distinct responses: an empty list (no error) for pages with zero rules, versus a DotSecurityException for pages with at least one rule. Over a set of page identifiers this allows probing which pages have rules configured without holding VIEW Rules permission.

final List<Rule> rules = rulesFactory.getAllRulesByParent(parent);
if (rules.isEmpty()) {
    return Collections.emptyList();   // permission check silently skipped
}
checkRulePermission(user, contentletParent, PermissionAPI.PERMISSION_READ, respectFrontendRoles);

💡 If the trade-off is accepted (restoring v5.x copy behavior), add a comment explaining why the check is intentionally deferred so future readers do not treat it as a bug.


[🟡 Medium] dotCMS/src/enterprise/java/com/dotcms/enterprise/rules/RulesAPIImpl.java:364-426

deleteRulesByParent and copyRulesByParent both delegate to getAllRulesByParent. With this change, an unauthorized user invoking either method on a page with no rules receives a silent success rather than a DotSecurityException. No data is mutated (the list is empty), but the API contract for write operations no longer consistently enforces authorization.

// Both write methods delegate here:
List<Rule> rulesByParent = getAllRulesByParent(parent, user, respectFrontendRoles);
for (Rule rule : rulesByParent) { // empty list → no iteration, no permission error
    deleteRule(rule, user, respectFrontendRoles);
}

💡 For write operations, add an explicit upfront permission check independent of rule count, e.g. resolve contentletParent and call checkRulePermission(user, contentletParent, PermissionAPI.PERMISSION_PUBLISH, respectFrontendRoles) before the getAllRulesByParent call.


Next steps

  • 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in RulesAPIImpl.java
  • Every new push triggers a fresh review automatically

View job run

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Copy workflow action on HTML pages requires View Rules even when page has no rules; error "does not have permissions to VIEW Rules"

1 participant