[ https://issues.apache.org/jira/browse/OAK-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17024919#comment-17024919 ]
Angela Schreiber commented on OAK-8855: --------------------------------------- [~kunal3112], probably i was mistaken regarding the removal of hidden items. as far as touching the rep:principalNames are concerned: i would suggest you actually change the values to make it less confusing and also add comments explaining why you are modifying the properties... essentially adding all the explaining you did here. this would also make the final assertions clearer as you can test it for the existing principals, which you are not changing effectively _and_ the modifications you made to the cug policy. also i would like you to add addtional ways to test the result i would suggest too use {{PermissionProvider}} obtained from the admin 'root' object to verify the effective permissions. since adding addtional verifications may result in a convoluted, complex test case, i would suggest the following: # move all of the setup you need including removal of the nested-cug property to a separate private method or to the setup method. essentially making this test class only about this very issue. # then you can create multiple test-methods to look at and verify the expected outcome from different angles: ## looking at the test-session (as you already do), ## looking at the effective permissions for a given principal (or set of principals) (obtain {{PermissionProvider}}from the root would allow you to do this){{ }} ## use {{RootProvider}} to obtain an read-only wrapper for the {{root}} object, which allows you actually to look at the hidden property and verify that it's being restored in the way you intended it to be restored (i.e. including the modifications). regarding naming of the class and the test, i would like you to apply the following minor changes: * the naming of class and method to reflect that you are testing 1 specific scenario, where the parent nested-cug property is being removed. * please also add a comment linking this test class to this very jira issue. one minor change: please make sure the formatting is consistent: e.g. \{{}finally {}} is lacking a space. please, adjust the test class accordingly and provide the patch here. thanks. > Permission evaluation of nodes broken after :nestedCug removed from parent > node > ------------------------------------------------------------------------------- > > Key: OAK-8855 > URL: https://issues.apache.org/jira/browse/OAK-8855 > Project: Jackrabbit Oak > Issue Type: Bug > Components: authorization-cug > Affects Versions: 1.8.7 > Reporter: Kunal Shubham > Assignee: Angela Schreiber > Priority: Major > Attachments: 0001-Fix-nestedcug-permission-issue.patch, > OAK-8855_backport.patch > > > Steps to Reproduce: > # Create a node 'a' which has two children nodes 'b1' and 'b2'. The content > tree looks as shown: /content/a/b1, /content/a/b2. Create two users user1 and > user2. > # Apply CUG policy on /content/a. > ** Authorize user1 and user2 to read /content/a. > ** Authorize user1 to read /content/a/b1. > ** Authorize user2 to read /content/a/b2. > # Remove :nestedCugs property from /content/a/rep:cugPolicy. > # Create a content session, login with user2. Try to read /content/a/b1. > *Observed behavior* : user2 is able to read /content/a/b1. > *Expected behavior* : user2 should not be able to read /content/a/b1 as it is > unauthorized to do so. > Please note that :nestedCugs is removed by a mechanism which completely > overwrites content tree below "/content/a". -- This message was sent by Atlassian Jira (v8.3.4#803005)