enapps-enorman edited a comment 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)
   
   Also, I didn't intend to suggest that the properties defined by the mixin 
were the only properties that could be set, only that those properties should 
not be excluded from the get calls.


-- 
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]


Reply via email to