anchela commented on code in PR #2868:
URL: https://github.com/apache/jackrabbit-oak/pull/2868#discussion_r3166539681
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserProvider.java:
##########
@@ -361,4 +371,39 @@ private String getNodeName(@NotNull String authorizableId)
{
AuthorizableNodeName generator =
requireNonNull(config.getConfigValue(PARAM_AUTHORIZABLE_NODE_NAME,
AuthorizableNodeName.DEFAULT, AuthorizableNodeName.class));
return generator.generateNodeName(authorizableId);
}
+
+ private Tree createAuthorizableNodeAtAbsolutePath(@NotNull String
authorizableId,
+ @NotNull String ntName,
+ @NotNull String
absoluteOakPath) throws RepositoryException {
+ String nodeName = PathUtils.getName(absoluteOakPath);
+ String parentPath = PathUtils.getParentPath(absoluteOakPath);
+ String authRoot = NT_REP_GROUP.equals(ntName) ? groupPath : userPath;
+
+ if (!parentPath.startsWith(authRoot)) {
+ throw new ConstraintViolationException("Attempt to create
authorizable at '" + absoluteOakPath
+ + "' outside of the configured root '" + authRoot + "'");
+ }
+
+ Tree tree = root.getTree(parentPath);
+ while (!tree.isRoot() && !tree.exists()) {
+ tree = tree.getParent();
+ }
+ if (!tree.exists()) {
+ throw new AccessDeniedException("Missing permission to create
intermediate authorizable folders.");
Review Comment:
the exception message may be a bit misleading. it's actually that read
permission is lacking to read an existing parent path, no? ((note: i am seeing
this now but i may be consistent with the existing code)). maybe still worth
doing better here: "Unable to retrieve authorizable parent folder" (or
something like that)
##########
oak-jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/UserManager.java:
##########
@@ -305,6 +333,43 @@ User createUser(@NotNull String userID, @Nullable String
password, @NotNull Prin
@NotNull
Group createGroup(@NotNull String groupID, @NotNull Principal principal,
@Nullable String intermediatePath) throws AuthorizableExistsException,
RepositoryException;
+ /**
+ * Creates a new {@code Group} at the specified absolute JCR repository
path.
+ * <p>
+ * Unlike {@link #createGroup(String, Principal, String)} where the
+ * {@code intermediatePath} is a relative hint that implementations may
ignore,
+ * this method requires the caller to supply the exact absolute JCR path
at which
+ * the group node must be created. The path must start with {@code /} and
must
+ * be within the configured group root.
+ * <p>
+ * If a node already exists at the resolved location, the implementation
may
+ * append a numeric suffix to the node name to avoid the collision.
+ * <p>
+ * If the last segment of the resolved node name violates the contract of
the
+ * configured {@code AuthorizableNodeName}, a {@link
javax.jcr.nodetype.ConstraintViolationException}
+ * is thrown.
+ * <p>
+ * Implementations that cannot honour an arbitrary absolute path must throw
+ * {@link UnsupportedRepositoryOperationException}.
+ *
+ * @param groupID The ID of the new group.
+ * @param principal The principal of the new group.
+ * @param absolutePath The absolute JCR repository path at which the group
node
+ * must be created. Must not be {@code null}.
+ * @return The new {@code Group}.
+ * @throws AuthorizableExistsException if an authorizable with
the given
+ * groupID or principal
already exists.
+ * @throws UnsupportedRepositoryOperationException if the implementation
does not
+ * support creation at an
absolute path.
+ * @throws RepositoryException If another error occurs.
+ */
+ @NotNull
+ default Group createGroupWithAbsolutePath(@NotNull String groupID,
@NotNull Principal principal,
+ @NotNull String absolutePath)
+ throws AuthorizableExistsException,
UnsupportedRepositoryOperationException, RepositoryException {
+ return createGroup(groupID, principal, absolutePath);
Review Comment:
calling the method that takes an intermediate path will not yield the
desired result, no? maybe throwing "unsupportedrepositoryoperationexception"
would be more aligned with the api contract. wdyt?
##########
oak-jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/UserManager.java:
##########
@@ -213,6 +213,34 @@ User createUser(@NotNull String userID, @Nullable String
password, @NotNull Prin
@Nullable String intermediatePath) throws
AuthorizableExistsException, RepositoryException;
+ /**
+ * Creates a user for the given parameters at the specified absolute JCR
path.
+ * Unlike {@link #createUser(String, String, Principal, String)} where the
+ * {@code intermediatePath} is a relative hint, this method accepts an
absolute
+ * JCR repository path that precisely determines the location of the new
user node.
+ * <p>
+ * Implementations that do not support placement at an arbitrary absolute
path
+ * should throw {@link UnsupportedRepositoryOperationException}.
+ *
+ * @param userID The ID of the new user.
+ * @param password The initial password of the new user, may be {@code
null}.
+ * @param principal The principal of the new user.
+ * @param absolutePath The absolute JCR repository path at which the user
node
+ * must be created. Must not be {@code null}.
+ * @return The new {@code User}.
+ * @throws AuthorizableExistsException if an authorizable with
the given
+ * userID or principal
already exists.
+ * @throws UnsupportedRepositoryOperationException if the implementation
does not
+ * support creation at an
absolute path.
+ * @throws RepositoryException If another error occurs.
+ */
+ @NotNull
+ default User createUserWithAbsolutePath(@NotNull String userID, @Nullable
String password,
+ @NotNull Principal principal,
@NotNull String absolutePath)
+ throws AuthorizableExistsException,
UnsupportedRepositoryOperationException, RepositoryException {
+ return createUser(userID, password, principal, absolutePath);
Review Comment:
see comment with createGroupWithAbsolute path below. same here
##########
oak-jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/UserManager.java:
##########
@@ -305,6 +333,43 @@ User createUser(@NotNull String userID, @Nullable String
password, @NotNull Prin
@NotNull
Group createGroup(@NotNull String groupID, @NotNull Principal principal,
@Nullable String intermediatePath) throws AuthorizableExistsException,
RepositoryException;
+ /**
+ * Creates a new {@code Group} at the specified absolute JCR repository
path.
+ * <p>
+ * Unlike {@link #createGroup(String, Principal, String)} where the
+ * {@code intermediatePath} is a relative hint that implementations may
ignore,
+ * this method requires the caller to supply the exact absolute JCR path
at which
+ * the group node must be created. The path must start with {@code /} and
must
+ * be within the configured group root.
+ * <p>
+ * If a node already exists at the resolved location, the implementation
may
+ * append a numeric suffix to the node name to avoid the collision.
+ * <p>
+ * If the last segment of the resolved node name violates the contract of
the
Review Comment:
the current implementation does not do that. so, i would recommend to state
"an ConstraintViolationException may be thrown".
or if we want to keep that as a mandate then the impl should verify... but
there is currently no API in AuthorizableNodeName to verify a given name...
this would need to be added.
either way fine with me.
##########
oak-jackrabbit-api/src/main/java/org/apache/jackrabbit/api/security/user/UserManager.java:
##########
@@ -305,6 +333,43 @@ User createUser(@NotNull String userID, @Nullable String
password, @NotNull Prin
@NotNull
Group createGroup(@NotNull String groupID, @NotNull Principal principal,
@Nullable String intermediatePath) throws AuthorizableExistsException,
RepositoryException;
+ /**
+ * Creates a new {@code Group} at the specified absolute JCR repository
path.
+ * <p>
+ * Unlike {@link #createGroup(String, Principal, String)} where the
+ * {@code intermediatePath} is a relative hint that implementations may
ignore,
+ * this method requires the caller to supply the exact absolute JCR path
at which
+ * the group node must be created. The path must start with {@code /} and
must
+ * be within the configured group root.
+ * <p>
+ * If a node already exists at the resolved location, the implementation
may
Review Comment:
i am not 100% convinced about this option to add a suffix. this reads like a
contradiction to the mandate that the resulting group node has indeed the
specified path. wdyt?
--
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]
