Security: validate path-segment identifiers (path injection)

Changelog.md

@@ -11,6 +11,9 @@
   connector using `trustAllCertificates(true)` can no longer cause a later,
   secure connector to run with certificate validation disabled. Client creation
   is now also thread-safe.
+- Security: reject path separators in user/group identifiers used as URL path
+  segments (user and group provisioning, group folder group names) to prevent
+  URL/path injection
 - Add support for the Group Folders app: create, rename, delete and list group
   folders, grant/revoke group access, set group permissions and set the folder
   quota via the new `GroupFolders` connector and `NextcloudConnector` methods

src/main/java/org/aarboard/nextcloud/api/groupfolders/GroupFolders.java

@@ -135,7 +135,7 @@ public void revokeAccess(int groupFolderId, String group) {
 
     public CompletableFuture<XMLAnswer> revokeAccessAsync(int groupFolderId, String group) {
         return connectorCommon.executeDelete(GROUP_FOLDERS_ROOT,
-                String.format("%d/groups/%s", groupFolderId, group),
+                String.format("%d/groups/%s", groupFolderId, ConnectorCommon.requireValidPathSegment(group)),
                 XMLAnswerParser.getInstance(XMLAnswer.class));
     }
 
@@ -154,7 +154,8 @@ public void setGroupPermissions(int groupFolderId, String group, int permissions
     public CompletableFuture<XMLAnswer> setGroupPermissionsAsync(int groupFolderId, String group, int permissions) {
         List<NameValuePair> postParams = new ArrayList<>();
         postParams.add(new BasicNameValuePair("permissions", String.valueOf(permissions)));
-        String url = String.format("%s/%d/groups/%s", GROUP_FOLDERS_ROOT, groupFolderId, group);
+        String url = String.format("%s/%d/groups/%s", GROUP_FOLDERS_ROOT, groupFolderId,
+                ConnectorCommon.requireValidPathSegment(group));
         return connectorCommon.executePost(url, postParams, XMLAnswerParser.getInstance(XMLAnswer.class));
     }
 

src/main/java/org/aarboard/nextcloud/api/provisioning/ProvisionConnector.java

@@ -95,7 +95,7 @@ public boolean deleteUser(String userId) {
      */
     public CompletableFuture<JsonVoidAnswer> deleteUserAsync(String userId)
     {
-        return connectorCommon.executeDelete(USERS_PART, userId, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executeDelete(USERS_PART, ConnectorCommon.requireValidPathSegment(userId), JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -201,7 +201,7 @@ public User getUser(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<UserDetailsAnswer> getUserAsync(String userId) {
-        return connectorCommon.executeGet(USERS_PART+"/"+userId, JsonAnswerParser.getInstance(UserDetailsAnswer.class));
+        return connectorCommon.executeGet(USERS_PART+"/"+ConnectorCommon.requireValidPathSegment(userId), JsonAnswerParser.getInstance(UserDetailsAnswer.class));
     }
 
     /**
@@ -246,7 +246,7 @@ public CompletableFuture<JsonVoidAnswer> editUserAsync(String userId, UserData k
         List<NameValuePair> queryParams= new LinkedList<>();
         queryParams.add(new BasicNameValuePair("key", key.name().toLowerCase()));
         queryParams.add(new BasicNameValuePair("value", value));
-        return connectorCommon.executePut(USERS_PART, userId, queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePut(USERS_PART, ConnectorCommon.requireValidPathSegment(userId), queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -266,7 +266,7 @@ public boolean enableUser(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonVoidAnswer> enableUserAsync(String userId) {
-        return connectorCommon.executePut(USERS_PART, userId + "/enable", null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePut(USERS_PART, ConnectorCommon.requireValidPathSegment(userId) + "/enable", null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -286,7 +286,7 @@ public boolean disableUser(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonVoidAnswer> disableUserAsync(String userId) {
-        return connectorCommon.executePut(USERS_PART, userId + "/disable", null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePut(USERS_PART, ConnectorCommon.requireValidPathSegment(userId) + "/disable", null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -306,7 +306,7 @@ public List<String> getGroupsOfUser(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<GroupListAnswer> getGroupsOfUserAsync(String userId) {
-        return connectorCommon.executeGet(USERS_PART + "/" + userId + "/groups", null, JsonAnswerParser.getInstance(GroupListAnswer.class));
+        return connectorCommon.executeGet(USERS_PART + "/" + ConnectorCommon.requireValidPathSegment(userId) + "/groups", null, JsonAnswerParser.getInstance(GroupListAnswer.class));
     }
 
     /**
@@ -330,7 +330,7 @@ public boolean addUserToGroup(String userId, String groupId) {
     public CompletableFuture<JsonVoidAnswer> addUserToGroupAsync(String userId, String groupId) {
         List<NameValuePair> queryParams = new LinkedList<>();
         queryParams.add(new BasicNameValuePair(GROUPID_KEY, groupId));
-        return connectorCommon.executePost(USERS_PART + "/" + userId + "/groups", queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePost(USERS_PART + "/" + ConnectorCommon.requireValidPathSegment(userId) + "/groups", queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -354,7 +354,7 @@ public boolean removeUserFromGroup(String userId, String groupId) {
     public CompletableFuture<JsonVoidAnswer> removeUserFromGroupAsync(String userId, String groupId) {
         List<NameValuePair> queryParams = new LinkedList<>();
         queryParams.add(new BasicNameValuePair(GROUPID_KEY, groupId));
-        return connectorCommon.executeDelete(USERS_PART, userId + "/groups", queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executeDelete(USERS_PART, ConnectorCommon.requireValidPathSegment(userId) + "/groups", queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -374,7 +374,7 @@ public List<String> getSubadminGroupsOfUser(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonListAnswer> getSubadminGroupsOfUserAsync(String userId) {
-        return connectorCommon.executeGet(USERS_PART + "/" + userId + SUBADMINS_PART, null, JsonAnswerParser.getInstance(JsonListAnswer.class));
+        return connectorCommon.executeGet(USERS_PART + "/" + ConnectorCommon.requireValidPathSegment(userId) + SUBADMINS_PART, null, JsonAnswerParser.getInstance(JsonListAnswer.class));
     }
 
     /**
@@ -398,7 +398,7 @@ public boolean promoteToSubadmin(String userId, String groupId) {
     public CompletableFuture<JsonVoidAnswer> promoteToSubadminAsync(String userId, String groupId) {
         List<NameValuePair> queryParams = new LinkedList<>();
         queryParams.add(new BasicNameValuePair(GROUPID_KEY, groupId));
-        return connectorCommon.executePost(USERS_PART + "/" + userId + SUBADMINS_PART, queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePost(USERS_PART + "/" + ConnectorCommon.requireValidPathSegment(userId) + SUBADMINS_PART, queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -422,7 +422,7 @@ public boolean demoteSubadmin(String userId, String groupId) {
     public CompletableFuture<JsonVoidAnswer> demoteSubadminAsync(String userId, String groupId) {
         List<NameValuePair> queryParams = new LinkedList<>();
         queryParams.add(new BasicNameValuePair(GROUPID_KEY, groupId));
-        return connectorCommon.executeDelete(USERS_PART, userId + SUBADMINS_PART, queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executeDelete(USERS_PART, ConnectorCommon.requireValidPathSegment(userId) + SUBADMINS_PART, queryParams, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -442,7 +442,7 @@ public boolean sendWelcomeMail(String userId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonVoidAnswer> sendWelcomeMailAsync(String userId) {
-        return connectorCommon.executePost(USERS_PART + "/" + userId + "/welcome", JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executePost(USERS_PART + "/" + ConnectorCommon.requireValidPathSegment(userId) + "/welcome", JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**
@@ -462,7 +462,7 @@ public List<String> getMembersOfGroup(String groupId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<UserListAnswer> getMembersOfGroupAsync(String groupId) {
-        return connectorCommon.executeGet(GROUPS_PART + "/" + groupId + "/users", JsonAnswerParser.getInstance(UserListAnswer.class));
+        return connectorCommon.executeGet(GROUPS_PART + "/" + ConnectorCommon.requireValidPathSegment(groupId) + "/users", JsonAnswerParser.getInstance(UserListAnswer.class));
     }
 
     /**
@@ -482,7 +482,7 @@ public List<User> getMembersDetailsOfGroup(String groupId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<UserDetailsListAnswer> getMembersDetailsOfGroupAsync(String groupId) {
-        return connectorCommon.executeGet(GROUPS_PART + "/" + groupId + "/users/details", JsonAnswerParser.getInstance(UserDetailsListAnswer.class));
+        return connectorCommon.executeGet(GROUPS_PART + "/" + ConnectorCommon.requireValidPathSegment(groupId) + "/users/details", JsonAnswerParser.getInstance(UserDetailsListAnswer.class));
     }
 
     /**
@@ -502,7 +502,7 @@ public List<String> getSubadminsOfGroup(String groupId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonListAnswer> getSubadminsOfGroupAsync(String groupId) {
-        return connectorCommon.executeGet(GROUPS_PART + "/" + groupId + SUBADMINS_PART, null, JsonAnswerParser.getInstance(JsonListAnswer.class));
+        return connectorCommon.executeGet(GROUPS_PART + "/" + ConnectorCommon.requireValidPathSegment(groupId) + SUBADMINS_PART, null, JsonAnswerParser.getInstance(JsonListAnswer.class));
     }
 
     /**
@@ -544,7 +544,7 @@ public boolean deleteGroup(String groupId) {
      * @return a CompletableFuture containing the result of the operation
      */
     public CompletableFuture<JsonVoidAnswer> deleteGroupAsync(String groupId) {
-        return connectorCommon.executeDelete(GROUPS_PART, groupId, null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
+        return connectorCommon.executeDelete(GROUPS_PART, ConnectorCommon.requireValidPathSegment(groupId), null, JsonAnswerParser.getInstance(JsonVoidAnswer.class));
     }
 
     /**

src/main/java/org/aarboard/nextcloud/api/utils/ConnectorCommon.java

@@ -58,6 +58,27 @@ public ConnectorCommon(ServerConfig serverConfig) {
         this.serverConfig = serverConfig;
     }
 
+    /**
+     * Ensures a value can be safely used as a single URL path segment. Nextcloud
+     * object identifiers (user ids, group ids, ...) cannot legitimately contain
+     * a path separator, so rejecting one here prevents URL/path injection. All
+     * other reserved characters are already percent-encoded by the URL builder.
+     *
+     * @param value the identifier to use as a path segment
+     * @return the value unchanged if it is safe
+     * @throws IllegalArgumentException if the value is null or contains a
+     *         path separator
+     */
+    public static String requireValidPathSegment(String value) {
+        if (value == null) {
+            throw new IllegalArgumentException("Path segment must not be null");
+        }
+        if (value.indexOf('/') >= 0 || value.indexOf('\\') >= 0) {
+            throw new IllegalArgumentException("Path segment must not contain a path separator: " + value);
+        }
+        return value;
+    }
+
     public <R> CompletableFuture<R> executeGet(String part, ResponseParser<R> parser) {
         return executeGet(part, null, parser);
     }

src/test/java/org/aarboard/nextcloud/api/utils/ConnectorCommonTest.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2026 a.schild
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.aarboard.nextcloud.api.utils;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link ConnectorCommon#requireValidPathSegment(String)}.
+ *
+ * @author a.schild
+ */
+public class ConnectorCommonTest {
+
+    @Test
+    public void testValidSegmentPassesThrough() {
+        assertEquals("john.doe", ConnectorCommon.requireValidPathSegment("john.doe"));
+        assertEquals("group with spaces", ConnectorCommon.requireValidPathSegment("group with spaces"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testForwardSlashRejected() {
+        ConnectorCommon.requireValidPathSegment("admin/../otheruser");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testBackslashRejected() {
+        ConnectorCommon.requireValidPathSegment("admin\\evil");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNullRejected() {
+        ConnectorCommon.requireValidPathSegment(null);
+    }
+}