Do not allow white space at the start or end of a group name

apps/provisioning_api/lib/Groups.php

@@ -136,6 +136,10 @@ public function addGroup($parameters) {
 			\OCP\Util::writeLog('provisioning_api', 'Group name not supplied', \OCP\Util::ERROR);
 			return new OC_OCS_Result(null, 101, 'Invalid group name');
 		}
+		if (\trim($groupId) !== $groupId) {
+			\OCP\Util::writeLog('provisioning_api', 'Group name must not start or end with white space', \OCP\Util::ERROR);
+			return new OC_OCS_Result(null, 101, 'Invalid group name');
+		}
 		// Check if it exists
 		if ($this->groupManager->groupExists($groupId)) {
 			return new OC_OCS_Result(null, 102);

apps/provisioning_api/tests/GroupsTest.php

@@ -333,11 +333,26 @@ public function testGetSubAdminsOfGroupEmptyList() {
 		$this->assertEquals([], $result->getData());
 	}
 
-	public function testAddGroupEmptyGroup() {
+	public function dataAddGroupWithInvalidName() {
+		return [
+			[''],
+			[' '],
+			[' white-space-at-start'],
+			['white-space-at-end '],
+			[' white-space-at-both-ends '],
+		];
+	}
+
+	/**
+	 * @dataProvider dataAddGroupWithInvalidName
+	 *
+	 * @param string $groupName
+	 */
+	public function testAddGroupWithInvalidName($groupName) {
 		$this->request
 			->method('getParam')
 			->with('groupid')
-			->willReturn('');
+			->willReturn($groupName);
 
 		$result = $this->api->addGroup([]);
 

changelog/unreleased/39540

@@ -0,0 +1,3 @@
+Bugfix: Prevent group names starting or ending with white space
+
+https://github.com/owncloud/core/pull/39540

lib/private/Group/Manager.php

@@ -210,7 +210,7 @@ public function groupExists($gid) {
 	 * @return \OC\Group\Group
 	 */
 	public function createGroup($gid) {
-		if ($gid === '' || $gid === null) {
+		if ($gid === '' || $gid === null || \trim($gid) !== $gid) {
 			return false;
 		} elseif ($group = $this->get($gid)) {
 			return $group;

tests/acceptance/features/apiProvisioningGroups-v1/addGroup.feature

@@ -154,28 +154,28 @@ Feature: add groups
     And the HTTP status code should be "401"
     And group "another-new-group" should not exist
 
-  @issue-39533 @skipOnOcV10
-  Scenario: admin creates a group that has white space at the end of the name
+
+  Scenario: admin tries to create a group that has white space at the end of the name
     When the administrator sends a group creation request for group "white-space-at-end " using the provisioning API
-    Then the OCS status code should be "100"
+    Then the OCS status code should be "101"
     And the HTTP status code should be "200"
-    And group "white-space-at-end " should exist
+    And group "white-space-at-end " should not exist
     And group "white-space-at-end" should not exist
 
 
-  Scenario: admin creates a group that has white space at the start of the name
+  Scenario: admin tries to create a group that has white space at the start of the name
     When the administrator sends a group creation request for group " white-space-at-start" using the provisioning API
-    Then the OCS status code should be "100"
+    Then the OCS status code should be "101"
     And the HTTP status code should be "200"
-    And group " white-space-at-start" should exist
+    And group " white-space-at-start" should not exist
     But group "white-space-at-start" should not exist
 
 
-  Scenario: admin creates a group that is a single space
+  Scenario: admin tries to create a group that is a single space
     When the administrator sends a group creation request for group " " using the provisioning API
-    Then the OCS status code should be "100"
+    Then the OCS status code should be "101"
     And the HTTP status code should be "200"
-    And group " " should exist
+    And group " " should not exist
 
 
   Scenario: admin tries to create a group that is the empty string

tests/acceptance/features/apiProvisioningGroups-v2/addGroup.feature

@@ -150,28 +150,28 @@ Feature: add groups
     And the HTTP status code should be "401"
     And group "another-new-group" should not exist
 
-  @issue-39533 @skipOnOcV10
-  Scenario: admin creates a group that has white space at the end of the name
+
+  Scenario: admin tries to create a group that has white space at the end of the name
     When the administrator sends a group creation request for group "white-space-at-end " using the provisioning API
-    Then the OCS status code should be "200"
-    And the HTTP status code should be "200"
-    And group "white-space-at-end " should exist
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "400"
+    And group "white-space-at-end " should not exist
     And group "white-space-at-end" should not exist
 
 
-  Scenario: admin creates a group that has white space at the start of the name
+  Scenario: admin tries to create a group that has white space at the start of the name
     When the administrator sends a group creation request for group " white-space-at-start" using the provisioning API
-    Then the OCS status code should be "200"
-    And the HTTP status code should be "200"
-    And group " white-space-at-start" should exist
-    But group "white-space-at-start" should not exist
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "400"
+    And group " white-space-at-start" should not exist
+    And group "white-space-at-start" should not exist
 
 
-  Scenario: admin creates a group that is a single space
+  Scenario: admin tries to create a group that is a single space
     When the administrator sends a group creation request for group " " using the provisioning API
-    Then the OCS status code should be "200"
-    And the HTTP status code should be "200"
-    And group " " should exist
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "400"
+    And group " " should not exist
 
 
   Scenario: admin tries to create a group that is the empty string

tests/acceptance/features/bootstrap/OccUsersGroupsContext.php

@@ -422,6 +422,12 @@ public function theAdministratorRetrievesTheUserReportUsingTheOccCommand():void
 	 * @throws Exception
 	 */
 	public function theAdministratorCreatesGroupUsingTheOccCommand(string $group):void {
+		if (($group === '') || (\trim($group) !== $group)) {
+			// The group name is empty or has white space at the start or end.
+			// Quote the group name so that the white space is really sent to the
+			// occ command as part of the requested group name.
+			$group = "'$group'";
+		}
 		$this->occContext->invokingTheCommand(
 			"group:add $group"
 		);

tests/acceptance/features/cliProvisioning/addGroup.feature

@@ -32,4 +32,29 @@ Feature: add group
     Given group "brand-new-group" has been created
     When the administrator creates group "brand-new-group" using the occ command
     Then the command should have failed with exit code 1
-    And the command output should contain the text 'The group "brand-new-group" already exists'
+    And the command output should contain the text 'The group "brand-new-group" already exists'
+
+  Scenario: admin tries to create a group that has white space at the end of the name
+    When the administrator creates group "white-space-at-end " using the occ command
+    Then the command should have failed with exit code 1
+    And the command output should contain the text 'Group "white-space-at-end " could not be created'
+    And group "white-space-at-end " should not exist
+    And group "white-space-at-end" should not exist
+
+  Scenario: admin tries to create a group that has white space at the start of the name
+    When the administrator creates group " white-space-at-start" using the occ command
+    Then the command should have failed with exit code 1
+    And the command output should contain the text 'Group " white-space-at-start" could not be created'
+    And group " white-space-at-start" should not exist
+    And group "white-space-at-start" should not exist
+
+  Scenario: admin tries to create a group that is a single space
+    When the administrator creates group " " using the occ command
+    Then the command should have failed with exit code 1
+    And the command output should contain the text 'Group " " could not be created'
+    And group " " should not exist
+
+  Scenario: admin tries to create a group that is the empty string
+    When the administrator creates group "" using the occ command
+    Then the command should have failed with exit code 1
+    And the command output should contain the text 'Group "" could not be created'

tests/lib/Group/ManagerTest.php

@@ -243,6 +243,26 @@ public function testCreate() {
 		$this->assertEquals('group1', $group->getGID());
 	}
 
+	public function dataCreateGroupWithInvalidName() {
+		return [
+			[''],
+			[' '],
+			[' white-space-at-start'],
+			['white-space-at-end '],
+			[' white-space-at-both-ends '],
+		];
+	}
+
+	/**
+	 * @dataProvider dataCreateGroupWithInvalidName
+	 *
+	 * @param string $groupName
+	 */
+	public function testCreateInvalidGroupName(string $groupName) {
+		$group = $this->manager->createGroup($groupName);
+		$this->assertFalse($group);
+	}
+
 	public function testCreateWithDispatcher() {
 		/**
 		 * @var \PHPUnit\Framework\MockObject\MockObject | \OC\Group\Backend $backend