From 451c120c8e4d72a074f84144171bc94c59821690 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Thu, 25 Nov 2021 11:48:32 +0545 Subject: [PATCH] Do not allow white space at the start or end of a group name --- apps/provisioning_api/lib/Groups.php | 4 +++ apps/provisioning_api/tests/GroupsTest.php | 19 +++++++++++-- changelog/unreleased/39540 | 3 ++ lib/private/Group/Manager.php | 2 +- .../apiProvisioningGroups-v1/addGroup.feature | 20 ++++++------- .../apiProvisioningGroups-v2/addGroup.feature | 28 +++++++++---------- .../bootstrap/OccUsersGroupsContext.php | 6 ++++ .../features/cliProvisioning/addGroup.feature | 27 +++++++++++++++++- tests/lib/Group/ManagerTest.php | 20 +++++++++++++ 9 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 changelog/unreleased/39540 diff --git a/apps/provisioning_api/lib/Groups.php b/apps/provisioning_api/lib/Groups.php index 86112edd9e7c..eadee62df3c2 100644 --- a/apps/provisioning_api/lib/Groups.php +++ b/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); diff --git a/apps/provisioning_api/tests/GroupsTest.php b/apps/provisioning_api/tests/GroupsTest.php index b007f5052899..182a4143b6a0 100644 --- a/apps/provisioning_api/tests/GroupsTest.php +++ b/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([]); diff --git a/changelog/unreleased/39540 b/changelog/unreleased/39540 new file mode 100644 index 000000000000..6582f0366b72 --- /dev/null +++ b/changelog/unreleased/39540 @@ -0,0 +1,3 @@ +Bugfix: Prevent group names starting or ending with white space + +https://github.com/owncloud/core/pull/39540 diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index 1d232f677394..8ead8cc8f55a 100644 --- a/lib/private/Group/Manager.php +++ b/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; diff --git a/tests/acceptance/features/apiProvisioningGroups-v1/addGroup.feature b/tests/acceptance/features/apiProvisioningGroups-v1/addGroup.feature index 36f8f1484b62..1d0828ef0639 100644 --- a/tests/acceptance/features/apiProvisioningGroups-v1/addGroup.feature +++ b/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 diff --git a/tests/acceptance/features/apiProvisioningGroups-v2/addGroup.feature b/tests/acceptance/features/apiProvisioningGroups-v2/addGroup.feature index 8cac2caf7f4e..a95bf9d95706 100644 --- a/tests/acceptance/features/apiProvisioningGroups-v2/addGroup.feature +++ b/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 diff --git a/tests/acceptance/features/bootstrap/OccUsersGroupsContext.php b/tests/acceptance/features/bootstrap/OccUsersGroupsContext.php index 8fa102d48dc1..48c3f1266cbe 100644 --- a/tests/acceptance/features/bootstrap/OccUsersGroupsContext.php +++ b/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" ); diff --git a/tests/acceptance/features/cliProvisioning/addGroup.feature b/tests/acceptance/features/cliProvisioning/addGroup.feature index 2bdc8d80661f..96bfba9d7b87 100644 --- a/tests/acceptance/features/cliProvisioning/addGroup.feature +++ b/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' \ No newline at end of file + 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' diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 167520d35468..5e689ceb4b7c 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/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