OAK-10334: Node.addMixin() may overwrite existing mixins #1011
Conversation
Test reproducing issue with jcr:mixinTypes
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
Outdated
Show resolved
Hide resolved
Allow adding a mixin type even if session does not have permission to read jcr:mixinTypes. This is consistent with behaviour of Node.getMixinNodeTypes(). See also OAK-2441.
There was a problem hiding this comment.
+1, lgtm.
What I'm wondering however, is whether this needs to be extended to removeMixin. There is code in this PR that fixes this pattern (write-but-no-read-permission) within the addMixin part of updateMixin, however, I'm unsure if there couldn't be the same issue in the remove part of updateMixin. It looks like NodeDelegate.removeMixin has some check to see if the to-be-removed mixin is actually part of the current mixin list - and that check is IIUC done with the actual session permission. So in the write-but-no-read-permission case that would probably fail there. But if addMixin now becomes more tolerant, then wouldn't removeMixin also need to be more tolerant? (But if that'd be the case, we could also handle this in a separate ticket, hence still +1)
That's correct.
Yes, I agree, but I rather handle this in a separate issue. The impact is a bit different. Removing a mixin would fail and not result in an unintentional change like in the addMixin() case. |
|
Follow up issue regarding Node.removeMixin(String): OAK-10425 |
|
All tests are successful, except DocumentStoreIndexerIT. This is a known issue: https://issues.apache.org/jira/browse/OAK-10413 |
Test reproducing issue with jcr:mixinTypes