enapps-enorman commented on pull request #471: URL: https://github.com/apache/jackrabbit-oak/pull/471#issuecomment-1024505122
> also, the patch only changes the get of authorizable properties and not the set, imho the same limitations should be applied for the set. in addition we should discuss if/how the configured mixin types should be automatically added to the user node or the properties node in the subtree in order to make sure it is present without having to explicitly obtain the Node beforehand. this in particular applies to sequences of 'setProperty' with relative path and subsequent 'getProperty'. Please see my related comment at https://github.com/apache/jackrabbit-oak/pull/471#discussion_r794754078 I'm not sure it would be practical to try to automatically add a mixin type during the setProperty calls, as there could be multiple choices on which mixin to use. This is why I prefer to add the mixin to the home node in a custom ActionProvider when the user/group is created. Also this proposal doesn't change any behavior for authorizable properties defined for relative subnodes under the home node. Access to mixin defined properties on the subnodes wasn't restricted like was for properties on the root home folder. > > finally, there should be additional test for set-get sequences, setting/getting without mixin set (-> should not work), setting/getting with relative paths that store properties somewhere in the subtree and not directly below the user/group node. FYI: getting a mixin defined property without this fix was already covered by [AuthorizablePropertiesImplTest#testGetPropertyDeclaredByDifferentType](https://github.com/apache/jackrabbit-oak/blob/c424e1c179092594c365034c7b3f6eceb6cf2b42/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImplTest.java#L192) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
