[jira] [Commented] (OAK-923) Runtime exception while creating a group

["angela (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13716360#comment-13716360 ] 

angela commented on OAK-923:
----------------------------

thanks for reporting this issue and the patch. just a few comments:

regarding the test:
IMHO it should fail not succeed as you deny all permissions on the root (but with AccessDenied instead of IllegalState)

regarding the SecureNodeUtil and the modifications made to UserProvider:
i think the runtime exception can be circumvented much simpler without passing around the permission provider: the exception is raised during NodeUtil#addChild in case the new child does not exist after being created. i see 2 ways to address this:
- verify if the added child exists in the utility and throw AccessDeniedException if that's not the case
- change the MutableTree such that a NEW tree is always accessible (afaik this would be the backwards compatible solution)

regarding UserInitializer:
- i would suggest to make the workspace name accessible from RootImpl instead of keeping it an shadowing field
- the system subject is a singleton. i would avoid passing the subject to the constructor and keeping it in a field.
- ContentSession#getUserID is allowed to return null which IMO was appropriate in this case.
                
> Runtime exception while creating a group
> ----------------------------------------
>
>                 Key: OAK-923
>                 URL: https://issues.apache.org/jira/browse/OAK-923
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: core
>            Reporter: Antonio Sanso
>            Assignee: angela
>         Attachments: OAK-923-patch.txt, OAK-923-test.txt, OAK-923-test.txt
>
>
> Trying to create a group where the user has not  permission ends up in a runtime exception
> {code}
> java.lang.IllegalStateException: This tree does not exist
> 	at com.google.common.base.Preconditions.checkState(Preconditions.java:149)
> 	at org.apache.jackrabbit.oak.core.MutableTree.checkExists(MutableTree.java:381)
> 	at org.apache.jackrabbit.oak.core.MutableTree.setProperty(MutableTree.java:298)
> 	at org.apache.jackrabbit.oak.util.NodeUtil.setName(NodeUtil.java:226)
> 	at org.apache.jackrabbit.oak.util.NodeUtil.addChild(NodeUtil.java:108)
> 	at org.apache.jackrabbit.oak.util.NodeUtil.getOrAddChild(NodeUtil.java:115)
> 	at org.apache.jackrabbit.oak.security.user.UserProvider.createFolderNodes(UserProvider.java:280)
> 	at org.apache.jackrabbit.oak.security.user.UserProvider.createAuthorizableNode(UserProvider.java:247)
> 	at org.apache.jackrabbit.oak.security.user.UserProvider.createGroup(UserProvider.java:185)
> 	at org.apache.jackrabbit.oak.security.user.UserManagerImpl.createGroup(UserManagerImpl.java:188)
> 	at org.apache.jackrabbit.oak.security.user.UserManagerImpl.createGroup(UserManagerImpl.java:166)
> 	at org.apache.jackrabbit.oak.jcr.security.authorization.UserManagementTest.testCreateGroup3(UserManagementTest.java:170)
> 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
> 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
> 	at java.lang.reflect.Method.invoke(Method.java:597)
> 	at junit.framework.TestCase.runTest(TestCase.java:168)
> 	at junit.framework.TestCase.runBare(TestCase.java:134)
> 	at junit.framework.TestResult$1.protect(TestResult.java:110)
> 	at junit.framework.TestResult.runProtected(TestResult.java:128)
> 	at junit.framework.TestResult.run(TestResult.java:113)
> 	at junit.framework.TestCase.run(TestCase.java:124)
> 	at org.apache.jackrabbit.test.AbstractJCRTest.run(AbstractJCRTest.java:464)
> 	at junit.framework.TestSuite.runTest(TestSuite.java:243)
> 	at junit.framework.TestSuite.run(TestSuite.java:238)
> 	at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83)
> 	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)
> 	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
> 	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
> 	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
> 	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
> 	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
> {code}
> It would be better to have a checked excpetion instead e.g. javax.jcr.AccessDeniedException
> I will attach test to reproduce the issue

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira