enapps-enorman commented on a change in pull request #471:
URL: https://github.com/apache/jackrabbit-oak/pull/471#discussion_r794741716
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
##########
@@ -234,9 +236,34 @@ private PropertyState getAuthorizableProperty(@NotNull
Tree authorizableTree, @N
}
ReadOnlyNodeTypeManager nodeTypeManager =
authorizable.getUserManager().getNodeTypeManager();
PropertyDefinition def = nodeTypeManager.getDefinition(parent,
property, true);
- if (def.isProtected() || (authorizablePath.equals(parent.getPath())
- &&
!def.getDeclaringNodeType().isNodeType(UserConstants.NT_REP_AUTHORIZABLE))) {
+ if (def.isProtected()) {
+ // exclude all protected properties
return null;
+ } else if (authorizablePath.equals(parent.getPath())) {
+ // non-protected properties on the root must be defined by the
expected
+ // primary type or one of the configured mixin types
+ Boolean allowed = null;
Review comment:
Sure, I'll make that cleanup change as the current impl only needs true
or false. This code was leftover from a failed experiment where a third state
would have been relevant.
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
##########
@@ -234,9 +236,34 @@ private PropertyState getAuthorizableProperty(@NotNull
Tree authorizableTree, @N
}
ReadOnlyNodeTypeManager nodeTypeManager =
authorizable.getUserManager().getNodeTypeManager();
PropertyDefinition def = nodeTypeManager.getDefinition(parent,
property, true);
- if (def.isProtected() || (authorizablePath.equals(parent.getPath())
- &&
!def.getDeclaringNodeType().isNodeType(UserConstants.NT_REP_AUTHORIZABLE))) {
+ if (def.isProtected()) {
+ // exclude all protected properties
return null;
+ } else if (authorizablePath.equals(parent.getPath())) {
+ // non-protected properties on the root must be defined by the
expected
+ // primary type or one of the configured mixin types
+ Boolean allowed = null;
+ NodeType declaringNodeType = def.getDeclaringNodeType();
+ if
(declaringNodeType.isNodeType(UserConstants.NT_REP_AUTHORIZABLE)) {
+ // defined by the expected primary type so allowed
+ allowed = Boolean.TRUE;
+ } else {
+ // OAK-9675 - not declared by the primary type, so let's check
for a mixin
+ if (declaringNodeType.isMixin()) {
+ // check if the property is defined by any of the
configured mixin type
+ String[] allowedMixins =
authorizable.getUserManager().getConfig().getConfigValue(UserConstants.PARAM_AUTHORIZABLE_PROPERTIES_MIXINS,
null, String[].class);
+ if (allowedMixins != null) {
+ // check if the actual mixin name is one of the
configured items
+ String actualMixinName = declaringNodeType.getName();
+ allowed =
Stream.of(allowedMixins).anyMatch(actualMixinName::equals);
+ }
+ }
+ }
+
+ if (!Boolean.TRUE.equals(allowed)) {
Review comment:
ok
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
##########
@@ -176,6 +176,13 @@
name = "RFC7613 Username Comparison Profile",
description = "Enable the UsercaseMappedProfile defined in
RFC7613 for username comparison.")
boolean enableRFC7613UsercaseMappedProfile() default false;
+
+ // for OAK-9675
+ @AttributeDefinition(
+ name = "Authorizable Properties Mixin Types",
+ description = "Optional configuration defining the names of
additional " +
Review comment:
Fair enough, I'll change "additional" to "any" to provide better clarity.
--
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]
