[MAGNOLIA-3899] PermissionUtil is too lax when converting permissions; it doesn't take into account custom permissions Created: 02/Dec/11  Updated: 09/Mar/12  Resolved: 20/Jan/12

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: None
Fix Version/s: 4.5

Type: Bug Priority: Blocker
Reporter: Magnolia International Assignee: Jan Haderka
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
causality
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

In the forum module, for example, we added a specific "permission" (moderation). Its code is 64. When doing PermissionUtil.convertPermissions(64), we get an empty string, which is probably going to get the code further down to check for no permission instead of checking for the one we want. I don't suppose JCR/JackRabbit can handle custom permissions, so these conversion methods should probably just throw an exception.

Consider the following test:

    @Test
    public void doesNotAccountForCustomPermissions() {
        try {
            final String result = PermissionUtil.convertPermissions(64);
            fail("Should have failed - but returned ["+result+"] instead.");
        } catch (IllegalArgumentException e) {
            assertEquals("64 is not a standard permission code, please update your code to use XYZ instead.", e.getMessage());
        }
    }

Setting the priority to blocker to make sure this gets fixed before the release. Please reset to an appropriate priority when fixed.



 Comments   
Comment by Jan Haderka [ 18/Jan/12 ]

Actually this method was never intended for converting all permissions, specially as custom permissions do not have corresponding JCR permission. Which is a reason why for such custom permissions method returned empty string.
In general custom permissions should be checked against AccessManager not against the content. Check for custom (64) permission was correctly returning no corresponding action, check for combination of custom and other permission (64+Permission.READ) correctly returned JCR action_read permission.
Naming of the method and javadoc need to be updated to remove this confusion. For the time being we might also output warning in the log file about checking no JCR permission against JCR, however throwing exception without giving option to separate custom and JCR permissions is not correct.

Generated at Mon Feb 12 03:50:45 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.