From 928138f11c2b30174db092dda1fad9d119cef92d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Mar 2016 16:46:27 +0100 Subject: [PATCH 1/5] Deduplicate the existing background --- build/integration/features/sharees.feature | 58 ++++++---------------- 1 file changed, 14 insertions(+), 44 deletions(-) diff --git a/build/integration/features/sharees.feature b/build/integration/features/sharees.feature index 35a80e7206..996a2b444c 100644 --- a/build/integration/features/sharees.feature +++ b/build/integration/features/sharees.feature @@ -1,12 +1,12 @@ Feature: sharees Background: Given using api version "1" - - Scenario: Search without exact match - Given user "test" exists + And user "test" exists And user "Sharee1" exists And group "ShareeGroup" exists - And As an "test" + + Scenario: Search without exact match + Given As an "test" When getting sharees for | search | Sharee | | itemType | file | @@ -22,10 +22,7 @@ Feature: sharees And "remotes" sharees returned is empty Scenario: Search without exact match not-exact casing - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | sharee | | itemType | file | @@ -42,10 +39,7 @@ Feature: sharees # TODO need to move the appconfig setting from Capabilities to Basic/Provisioning # Scenario: Search without exact match no iteration allowed -# Given user "test" exists -# And user "Sharee1" exists -# And group "ShareeGroup" exists -# And As an "test" +# Given As an "test" # When getting sharees for # | search | Sharee | # | itemType | file | @@ -59,10 +53,7 @@ Feature: sharees # And "remotes" sharees returned is empty # # Scenario: Search with exact match no iteration allowed -# Given user "test" exists -# And user "Sharee1" exists -# And group "ShareeGroup" exists -# And As an "test" +# Given As an "test" # When getting sharees for # | search | Sharee1 | # | itemType | file | @@ -77,10 +68,7 @@ Feature: sharees # And "remotes" sharees returned is empty # # Scenario: Search with exact match group no iteration allowed -# Given user "test" exists -# And user "Sharee1" exists -# And group "ShareeGroup" exists -# And As an "test" +# Given As an "test" # When getting sharees for # | search | ShareeGroup | # | itemType | file | @@ -95,10 +83,7 @@ Feature: sharees # And "remotes" sharees returned is empty Scenario: Search with exact match - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | Sharee1 | | itemType | file | @@ -113,10 +98,7 @@ Feature: sharees Then "remotes" sharees returned is empty Scenario: Search with exact match not-exact casing - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | sharee1 | | itemType | file | @@ -131,10 +113,7 @@ Feature: sharees Then "remotes" sharees returned is empty Scenario: Search with exact match not-exact casing group - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | shareegroup | | itemType | file | @@ -149,10 +128,7 @@ Feature: sharees Then "remotes" sharees returned is empty Scenario: Search with "self" - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "Sharee1" + Given As an "Sharee1" When getting sharees for | search | Sharee1 | | itemType | file | @@ -167,10 +143,7 @@ Feature: sharees Then "remotes" sharees returned is empty Scenario: Remote sharee for files - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | test@localhost | | itemType | file | @@ -185,10 +158,7 @@ Feature: sharees Then "remotes" sharees returned is empty Scenario: Remote sharee for calendars not allowed - Given user "test" exists - And user "Sharee1" exists - And group "ShareeGroup" exists - And As an "test" + Given As an "test" When getting sharees for | search | test@localhost | | itemType | calendar | From 8eb963849073923bfbd0e79e9e6c4f21812b7c43 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 15 Mar 2016 14:23:41 +0100 Subject: [PATCH 2/5] Make the app config code ready for reuse --- .../features/bootstrap/AppConfiguration.php | 103 ++++++++++++++++++ .../bootstrap/CapabilitiesContext.php | 79 +------------- 2 files changed, 104 insertions(+), 78 deletions(-) create mode 100644 build/integration/features/bootstrap/AppConfiguration.php diff --git a/build/integration/features/bootstrap/AppConfiguration.php b/build/integration/features/bootstrap/AppConfiguration.php new file mode 100644 index 0000000000..af904a3089 --- /dev/null +++ b/build/integration/features/bootstrap/AppConfiguration.php @@ -0,0 +1,103 @@ +currentUser; + $this->currentUser = 'admin'; + + $this->modifyServerConfig($app, $parameter, $value); + + $this->currentUser = $user; + } + + /** + * @param string $app + * @param string $parameter + * @param string $value + */ + protected function modifyServerConfig($app, $parameter, $value) { + $body = new \Behat\Gherkin\Node\TableNode([['value', $value]]); + $this->sendingToWith('post', "/apps/testing/api/v1/app/{$app}/{$parameter}", $body); + $this->theHTTPStatusCodeShouldBe('200'); + $this->theOCSStatusCodeShouldBe('100'); + } + + protected function setStatusTestingApp($enabled) { + $this->sendingTo(($enabled ? 'post' : 'delete'), '/cloud/apps/testing'); + $this->theHTTPStatusCodeShouldBe('200'); + $this->theOCSStatusCodeShouldBe('100'); + + $this->sendingTo('get', '/cloud/apps?filter=enabled'); + $this->theHTTPStatusCodeShouldBe('200'); + if ($enabled) { + PHPUnit_Framework_Assert::assertContains('testing', $this->response->getBody()->getContents()); + } else { + PHPUnit_Framework_Assert::assertNotContains('testing', $this->response->getBody()->getContents()); + } + } + + abstract protected function resetAppConfigs(); + + /** + * @BeforeScenario + * + * Enable the testing app before the first scenario of the feature and + * reset the configs before each scenario + * @param BeforeScenarioScope $event + */ + public function prepareParameters(BeforeScenarioScope $event){ + $user = $this->currentUser; + $this->currentUser = 'admin'; + + $scenarios = $event->getFeature()->getScenarios(); + if ($event->getScenario() === reset($scenarios)) { + $this->setStatusTestingApp(true); + } + + $this->resetAppConfigs(); + + $this->currentUser = $user; + } + + /** + * @AfterScenario + * + * Reset the values after the last scenario of the feature and disable the testing app + * @param AfterScenarioScope $event + */ + public function undoChangingParameters(AfterScenarioScope $event) { + $scenarios = $event->getFeature()->getScenarios(); + if ($event->getScenario() === end($scenarios)) { + $user = $this->currentUser; + $this->currentUser = 'admin'; + + $this->resetAppConfigs(); + + $this->setStatusTestingApp(false); + $this->currentUser = $user; + } + } +} diff --git a/build/integration/features/bootstrap/CapabilitiesContext.php b/build/integration/features/bootstrap/CapabilitiesContext.php index 0eb6353a89..91a4265504 100644 --- a/build/integration/features/bootstrap/CapabilitiesContext.php +++ b/build/integration/features/bootstrap/CapabilitiesContext.php @@ -15,18 +15,7 @@ require __DIR__ . '/../../vendor/autoload.php'; class CapabilitiesContext implements Context, SnippetAcceptingContext { use BasicStructure; - - /** - * @Given /^parameter "([^"]*)" of app "([^"]*)" is set to "([^"]*)"$/ - */ - public function serverParameterIsSetTo($parameter, $app, $value){ - $user = $this->currentUser; - $this->currentUser = 'admin'; - - $this->modifyServerConfig($app, $parameter, $value); - - $this->currentUser = $user; - } + use AppConfiguration; /** * @Then /^fields of capabilities match with$/ @@ -63,70 +52,4 @@ class CapabilitiesContext implements Context, SnippetAcceptingContext { $this->modifyServerConfig('core', 'shareapi_default_expire_date', 'no'); $this->modifyServerConfig('core', 'shareapi_enforce_expire_date', 'no'); } - - /** - * @BeforeScenario - * - * Enable the testing app before the first scenario of the feature and - * reset the configs before each scenario - * @param BeforeScenarioScope $event - */ - public function prepareParameters(BeforeScenarioScope $event){ - $user = $this->currentUser; - $this->currentUser = 'admin'; - - $scenarios = $event->getFeature()->getScenarios(); - if ($event->getScenario() === reset($scenarios)) { - $this->setStatusTestingApp(true); - } - - $this->resetAppConfigs(); - - $this->currentUser = $user; - } - - /** - * @AfterScenario - * - * Reset the values after the last scenario of the feature and disable the testing app - * @param AfterScenarioScope $event - */ - public function undoChangingParameters(AfterScenarioScope $event) { - $scenarios = $event->getFeature()->getScenarios(); - if ($event->getScenario() === end($scenarios)) { - $user = $this->currentUser; - $this->currentUser = 'admin'; - - $this->resetAppConfigs(); - - $this->setStatusTestingApp(false); - $this->currentUser = $user; - } - } - - /** - * @param string $app - * @param string $parameter - * @param string $value - */ - protected function modifyServerConfig($app, $parameter, $value) { - $body = new \Behat\Gherkin\Node\TableNode([['value', $value]]); - $this->sendingToWith('post', "/apps/testing/api/v1/app/{$app}/{$parameter}", $body); - $this->theHTTPStatusCodeShouldBe('200'); - $this->theOCSStatusCodeShouldBe('100'); - } - - protected function setStatusTestingApp($enabled) { - $this->sendingTo(($enabled ? 'post' : 'delete'), '/cloud/apps/testing'); - $this->theHTTPStatusCodeShouldBe('200'); - $this->theOCSStatusCodeShouldBe('100'); - - $this->sendingTo('get', '/cloud/apps?filter=enabled'); - $this->theHTTPStatusCodeShouldBe('200'); - if ($enabled) { - PHPUnit_Framework_Assert::assertContains('testing', $this->response->getBody()->getContents()); - } else { - PHPUnit_Framework_Assert::assertNotContains('testing', $this->response->getBody()->getContents()); - } - } } From 4e9c3b3d641d9f11a78420163dfad46c74e554b8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 15 Mar 2016 14:24:21 +0100 Subject: [PATCH 3/5] Always use the admin when putting poeple in a group --- .../features/bootstrap/Provisioning.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index bf84058c05..feeb850ae7 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -175,14 +175,15 @@ trait Provisioning { * @param string $group */ public function assureUserBelongsToGroup($user, $group){ - if (!$this->userBelongsToGroup($user, $group)){ - $previous_user = $this->currentUser; - $this->currentUser = "admin"; - $this->addingUserToGroup($user, $group); - $this->currentUser = $previous_user; - } - $this->checkThatUserBelongsToGroup($user, $group); + $previous_user = $this->currentUser; + $this->currentUser = "admin"; + if (!$this->userBelongsToGroup($user, $group)){ + $this->addingUserToGroup($user, $group); + } + + $this->checkThatUserBelongsToGroup($user, $group); + $this->currentUser = $previous_user; } /** From c4b0a1cdfd8d2c4961f1049b424c7655aef3a55e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 15 Mar 2016 14:24:51 +0100 Subject: [PATCH 4/5] Add tests for user enumeration and sharing in group only --- .../features/bootstrap/Sharing.php | 8 +- build/integration/features/sharees.feature | 140 ++++++++++++------ 2 files changed, 103 insertions(+), 45 deletions(-) diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index f516b97146..f81442c976 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -7,8 +7,9 @@ require __DIR__ . '/../../vendor/autoload.php'; -trait Sharing{ +trait Sharing { use Provisioning; + use AppConfiguration; /** @var int */ private $sharingApiVersion = 1; @@ -520,5 +521,10 @@ trait Sharing{ } return $sharees; } + + protected function resetAppConfigs() { + $this->modifyServerConfig('core', 'shareapi_only_share_with_group_members', 'no'); + $this->modifyServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'); + } } diff --git a/build/integration/features/sharees.feature b/build/integration/features/sharees.feature index 996a2b444c..5765b937a6 100644 --- a/build/integration/features/sharees.feature +++ b/build/integration/features/sharees.feature @@ -4,6 +4,7 @@ Feature: sharees And user "test" exists And user "Sharee1" exists And group "ShareeGroup" exists + And user "test" belongs to group "ShareeGroup" Scenario: Search without exact match Given As an "test" @@ -37,50 +38,101 @@ Feature: sharees And "exact remotes" sharees returned is empty And "remotes" sharees returned is empty -# TODO need to move the appconfig setting from Capabilities to Basic/Provisioning -# Scenario: Search without exact match no iteration allowed -# Given As an "test" -# When getting sharees for -# | search | Sharee | -# | itemType | file | -# Then the OCS status code should be "100" -# And the HTTP status code should be "200" -# And "exact users" sharees returned is empty -# And "users" sharees returned is empty -# And "exact groups" sharees returned is empty -# And "groups" sharees returned is empty -# And "exact remotes" sharees returned is empty -# And "remotes" sharees returned is empty -# -# Scenario: Search with exact match no iteration allowed -# Given As an "test" -# When getting sharees for -# | search | Sharee1 | -# | itemType | file | -# Then the OCS status code should be "100" -# And the HTTP status code should be "200" -# And "exact users" sharees returned are -# | Sharee1 | 0 | Sharee1 | -# And "users" sharees returned is empty -# And "exact groups" sharees returned is empty -# And "groups" sharees returned is empty -# And "exact remotes" sharees returned is empty -# And "remotes" sharees returned is empty -# -# Scenario: Search with exact match group no iteration allowed -# Given As an "test" -# When getting sharees for -# | search | ShareeGroup | -# | itemType | file | -# Then the OCS status code should be "100" -# And the HTTP status code should be "200" -# And "exact users" sharees returned is empty -# And "users" sharees returned is empty -# And "exact groups" sharees returned are -# | ShareeGroup | 1 | ShareeGroup | -# And "groups" sharees returned is empty -# And "exact remotes" sharees returned is empty -# And "remotes" sharees returned is empty + Scenario: Search only with group members - denied + Given As an "test" + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + When getting sharees for + | search | sharee | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned is empty + And "exact groups" sharees returned is empty + And "groups" sharees returned are + | ShareeGroup | 1 | ShareeGroup | + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty + + Scenario: Search only with group members - allowed + Given As an "test" + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + And user "Sharee1" belongs to group "ShareeGroup" + When getting sharees for + | search | sharee | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned are + | Sharee1 | 0 | Sharee1 | + And "exact groups" sharees returned is empty + And "groups" sharees returned are + | ShareeGroup | 1 | ShareeGroup | + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty + + Scenario: Search only with group members - no group as non-member + Given As an "Sharee1" + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + When getting sharees for + | search | sharee | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned is empty + And "exact groups" sharees returned is empty + And "groups" sharees returned is empty + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty + + Scenario: Search without exact match no iteration allowed + Given As an "test" + And parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + When getting sharees for + | search | Sharee | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned is empty + And "exact groups" sharees returned is empty + And "groups" sharees returned is empty + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty + + Scenario: Search with exact match no iteration allowed + Given As an "test" + And parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + When getting sharees for + | search | Sharee1 | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned are + | Sharee1 | 0 | Sharee1 | + And "users" sharees returned is empty + And "exact groups" sharees returned is empty + And "groups" sharees returned is empty + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty + + Scenario: Search with exact match group no iteration allowed + Given As an "test" + And parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + When getting sharees for + | search | ShareeGroup | + | itemType | file | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned is empty + And "exact groups" sharees returned are + | ShareeGroup | 1 | ShareeGroup | + And "groups" sharees returned is empty + And "exact remotes" sharees returned is empty + And "remotes" sharees returned is empty Scenario: Search with exact match Given As an "test" From cb56dfec6be42b46bb57c65eb2af8e005d60d8d6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 15 Mar 2016 14:53:16 +0100 Subject: [PATCH 5/5] Split the context so we don't reset the config on each test --- build/integration/config/behat.yml | 10 +++ .../features/bootstrap/ShareesContext.php | 72 +++++++++++++++++++ .../features/bootstrap/Sharing.php | 57 --------------- .../sharees.feature | 0 4 files changed, 82 insertions(+), 57 deletions(-) create mode 100644 build/integration/features/bootstrap/ShareesContext.php rename build/integration/{features => sharees_features}/sharees.feature (100%) diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index 4b5b5b16ef..0c0ecef08e 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -42,6 +42,16 @@ default: - admin - admin regular_user_password: 123456 + sharees: + paths: + - %paths.base%/../sharees_features + contexts: + - ShareesContext: + baseUrl: http://localhost:8080/ocs/ + admin: + - admin + - admin + regular_user_password: 123456 diff --git a/build/integration/features/bootstrap/ShareesContext.php b/build/integration/features/bootstrap/ShareesContext.php new file mode 100644 index 0000000000..de50eeb2be --- /dev/null +++ b/build/integration/features/bootstrap/ShareesContext.php @@ -0,0 +1,72 @@ +getRowsHash() as $key => $value) { + $parameters[] = $key . '=' . $value; + } + if (!empty($parameters)) { + $url .= '?' . implode('&', $parameters); + } + } + + $this->sendingTo('GET', $url); + } + + /** + * @Then /^"([^"]*)" sharees returned (are|is empty)$/ + * @param string $shareeType + * @param string $isEmpty + * @param \Behat\Gherkin\Node\TableNode|null $shareesList + */ + public function thenListOfSharees($shareeType, $isEmpty, $shareesList = null) { + if ($isEmpty !== 'is empty') { + $sharees = $shareesList->getRows(); + $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); + PHPUnit_Framework_Assert::assertEquals($sharees, $respondedArray); + } else { + $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); + PHPUnit_Framework_Assert::assertEmpty($respondedArray); + } + } + + public function getArrayOfShareesResponded(ResponseInterface $response, $shareeType) { + $elements = $response->xml()->data; + $elements = json_decode(json_encode($elements), 1); + if (strpos($shareeType, 'exact ') === 0) { + $elements = $elements['exact']; + $shareeType = substr($shareeType, 6); + } + + $sharees = []; + foreach ($elements[$shareeType] as $element) { + $sharees[] = [$element['label'], $element['value']['shareType'], $element['value']['shareWith']]; + } + return $sharees; + } + + protected function resetAppConfigs() { + $this->modifyServerConfig('core', 'shareapi_only_share_with_group_members', 'no'); + $this->modifyServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'); + } +} diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index f81442c976..d423a28f19 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -9,7 +9,6 @@ require __DIR__ . '/../../vendor/autoload.php'; trait Sharing { use Provisioning; - use AppConfiguration; /** @var int */ private $sharingApiVersion = 1; @@ -470,61 +469,5 @@ trait Sharing { throw new \Exception('Expected the same link share to be returned'); } } - - /** - * @When /^getting sharees for$/ - * @param \Behat\Gherkin\Node\TableNode $body - */ - public function whenGettingShareesFor($body) { - $url = '/apps/files_sharing/api/v1/sharees'; - if ($body instanceof \Behat\Gherkin\Node\TableNode) { - $parameters = []; - foreach ($body->getRowsHash() as $key => $value) { - $parameters[] = $key . '=' . $value; - } - if (!empty($parameters)) { - $url .= '?' . implode('&', $parameters); - } - } - - $this->sendingTo('GET', $url); - } - - /** - * @Then /^"([^"]*)" sharees returned (are|is empty)$/ - * @param string $shareeType - * @param string $isEmpty - * @param \Behat\Gherkin\Node\TableNode|null $shareesList - */ - public function thenListOfSharees($shareeType, $isEmpty, $shareesList = null) { - if ($isEmpty !== 'is empty') { - $sharees = $shareesList->getRows(); - $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); - PHPUnit_Framework_Assert::assertEquals($sharees, $respondedArray); - } else { - $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); - PHPUnit_Framework_Assert::assertEmpty($respondedArray); - } - } - - public function getArrayOfShareesResponded(ResponseInterface $response, $shareeType) { - $elements = $response->xml()->data; - $elements = json_decode(json_encode($elements), 1); - if (strpos($shareeType, 'exact ') === 0) { - $elements = $elements['exact']; - $shareeType = substr($shareeType, 6); - } - - $sharees = []; - foreach ($elements[$shareeType] as $element) { - $sharees[] = [$element['label'], $element['value']['shareType'], $element['value']['shareWith']]; - } - return $sharees; - } - - protected function resetAppConfigs() { - $this->modifyServerConfig('core', 'shareapi_only_share_with_group_members', 'no'); - $this->modifyServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'); - } } diff --git a/build/integration/features/sharees.feature b/build/integration/sharees_features/sharees.feature similarity index 100% rename from build/integration/features/sharees.feature rename to build/integration/sharees_features/sharees.feature