From 06e3708697cfc8b965e560805551bb9d083da824 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 3 Jun 2019 15:24:26 +0200 Subject: [PATCH 1/2] add LDAP integr. test for receiving share candidates with group limitation Signed-off-by: Arthur Schiwon --- build/integration/config/behat.yml | 2 +- .../features/bootstrap/AppConfiguration.php | 2 +- .../features/bootstrap/BasicStructure.php | 30 ++++++++- .../features/bootstrap/CalDavContext.php | 2 +- .../features/bootstrap/LDAPContext.php | 9 ++- .../features/bootstrap/ShareesContext.php | 53 +-------------- .../features/bootstrap/Sharing.php | 66 +++++++++++++------ .../integration/features/bootstrap/WebDav.php | 23 ------- .../openldap-uid-username.feature | 21 ++++++ 9 files changed, 106 insertions(+), 102 deletions(-) diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index 7fd3686b8bb..51b749360e3 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -84,7 +84,7 @@ default: admin: - admin - admin - regular_user_password: what_for + regular_user_password: 123456 remoteapi: paths: - "%paths.base%/../remoteapi_features" diff --git a/build/integration/features/bootstrap/AppConfiguration.php b/build/integration/features/bootstrap/AppConfiguration.php index faf80d85e11..d39c51e387a 100644 --- a/build/integration/features/bootstrap/AppConfiguration.php +++ b/build/integration/features/bootstrap/AppConfiguration.php @@ -23,8 +23,8 @@ */ use Behat\Behat\Hook\Scope\AfterScenarioScope; use Behat\Behat\Hook\Scope\BeforeScenarioScope; -use GuzzleHttp\Message\ResponseInterface; use PHPUnit\Framework\Assert; +use Psr\Http\Message\ResponseInterface; require __DIR__ . '/../../vendor/autoload.php'; diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index f6c93aa5174..3f4f70115bf 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -29,6 +29,7 @@ * */ +use Behat\Gherkin\Node\TableNode; use GuzzleHttp\Client; use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Exception\ClientException; @@ -165,7 +166,7 @@ trait BasicStructure { * @When /^sending "([^"]*)" to "([^"]*)" with$/ * @param string $verb * @param string $url - * @param \Behat\Gherkin\Node\TableNode $body + * @param TableNode $body */ public function sendingToWith($verb, $url, $body) { $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url; @@ -179,7 +180,7 @@ trait BasicStructure { $options['headers'] = [ 'OCS_APIREQUEST' => 'true' ]; - if ($body instanceof \Behat\Gherkin\Node\TableNode) { + if ($body instanceof TableNode) { $fd = $body->getRowsHash(); $options['form_params'] = $fd; } @@ -216,7 +217,7 @@ trait BasicStructure { } else { $options['auth'] = [$this->currentUser, $this->regularUser]; } - if ($body instanceof \Behat\Gherkin\Node\TableNode) { + if ($body instanceof TableNode) { $fd = $body->getRowsHash(); $options['form_params'] = $fd; } @@ -504,4 +505,27 @@ trait BasicStructure { public function cookiesAreReset() { $this->cookieJar = new CookieJar(); } + + /** + * @Then The following headers should be set + * @param TableNode $table + * @throws \Exception + */ + public function theFollowingHeadersShouldBeSet(TableNode $table) { + foreach($table->getTable() as $header) { + $headerName = $header[0]; + $expectedHeaderValue = $header[1]; + $returnedHeader = $this->response->getHeader($headerName)[0]; + if($returnedHeader !== $expectedHeaderValue) { + throw new \Exception( + sprintf( + "Expected value '%s' for header '%s', got '%s'", + $expectedHeaderValue, + $headerName, + $returnedHeader + ) + ); + } + } + } } diff --git a/build/integration/features/bootstrap/CalDavContext.php b/build/integration/features/bootstrap/CalDavContext.php index cbd29a4fbff..1c64c74c036 100644 --- a/build/integration/features/bootstrap/CalDavContext.php +++ b/build/integration/features/bootstrap/CalDavContext.php @@ -25,7 +25,7 @@ require __DIR__ . '/../../vendor/autoload.php'; use GuzzleHttp\Client; -use GuzzleHttp\Message\ResponseInterface; +use Psr\Http\Message\ResponseInterface; class CalDavContext implements \Behat\Behat\Context\Context { /** @var string */ diff --git a/build/integration/features/bootstrap/LDAPContext.php b/build/integration/features/bootstrap/LDAPContext.php index 2ad737bf8b8..4932eaa36a5 100644 --- a/build/integration/features/bootstrap/LDAPContext.php +++ b/build/integration/features/bootstrap/LDAPContext.php @@ -26,8 +26,9 @@ use Behat\Gherkin\Node\TableNode; use PHPUnit\Framework\Assert; class LDAPContext implements Context { - use BasicStructure; - use CommandLine; + use AppConfiguration, + CommandLine, + Sharing; // Pulls in BasicStructure protected $configID; @@ -204,4 +205,8 @@ class LDAPContext implements Context { $configKey = $this->configID . 'ldap_configuration_active'; $this->invokingTheCommand('config:app:set user_ldap ' . $configKey . ' --value="0"'); } + + protected function resetAppConfigs() { + // not implemented + } } diff --git a/build/integration/features/bootstrap/ShareesContext.php b/build/integration/features/bootstrap/ShareesContext.php index 41980e2cd21..ee4d9420581 100644 --- a/build/integration/features/bootstrap/ShareesContext.php +++ b/build/integration/features/bootstrap/ShareesContext.php @@ -33,60 +33,9 @@ require __DIR__ . '/../../vendor/autoload.php'; * Features context. */ class ShareesContext implements Context, SnippetAcceptingContext { - use Provisioning; + use Sharing; use AppConfiguration; - /** - * @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); - Assert::assertEquals($sharees, $respondedArray); - } else { - $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); - Assert::assertEmpty($respondedArray); - } - } - - public function getArrayOfShareesResponded(ResponseInterface $response, $shareeType) { - $elements = simplexml_load_string($response->getBody())->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 9a3c00af489..efacc6e4154 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -26,8 +26,8 @@ * */ use GuzzleHttp\Client; -use GuzzleHttp\Message\ResponseInterface; use PHPUnit\Framework\Assert; +use Psr\Http\Message\ResponseInterface; require __DIR__ . '/../../vendor/autoload.php'; @@ -45,7 +45,7 @@ trait Sharing { /** @var int */ private $savedShareId = null; - /** @var \Psr\Http\Message\ResponseInterface */ + /** @var ResponseInterface */ private $response; /** @@ -529,26 +529,54 @@ trait Sharing { } /** - * @Then The following headers should be set - * @param \Behat\Gherkin\Node\TableNode $table - * @throws \Exception + * @When /^getting sharees for$/ + * @param \Behat\Gherkin\Node\TableNode $body */ - public function theFollowingHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode $table) { - foreach($table->getTable() as $header) { - $headerName = $header[0]; - $expectedHeaderValue = $header[1]; - $returnedHeader = $this->response->getHeader($headerName)[0]; - if($returnedHeader !== $expectedHeaderValue) { - throw new \Exception( - sprintf( - "Expected value '%s' for header '%s', got '%s'", - $expectedHeaderValue, - $headerName, - $returnedHeader - ) - ); + 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); + Assert::assertEquals($sharees, $respondedArray); + } else { + $respondedArray = $this->getArrayOfShareesResponded($this->response, $shareeType); + Assert::assertEmpty($respondedArray); + } + } + + public function getArrayOfShareesResponded(ResponseInterface $response, $shareeType) { + $elements = simplexml_load_string($response->getBody())->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; } } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index b36cdfaea68..6bc808c3bcb 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -226,29 +226,6 @@ trait WebDav { } } - /** - * @Then The following headers should be set - * @param \Behat\Gherkin\Node\TableNode $table - * @throws \Exception - */ - public function theFollowingHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode $table) { - foreach ($table->getTable() as $header) { - $headerName = $header[0]; - $expectedHeaderValue = $header[1]; - $returnedHeader = $this->response->getHeader($headerName)[0]; - if ($returnedHeader !== $expectedHeaderValue) { - throw new \Exception( - sprintf( - "Expected value '%s' for header '%s', got '%s'", - $expectedHeaderValue, - $headerName, - $returnedHeader - ) - ); - } - } - } - /** * @Then Downloaded content should start with :start * @param int $start diff --git a/build/integration/ldap_features/openldap-uid-username.feature b/build/integration/ldap_features/openldap-uid-username.feature index e120d0316de..1790106ad56 100644 --- a/build/integration/ldap_features/openldap-uid-username.feature +++ b/build/integration/ldap_features/openldap-uid-username.feature @@ -118,3 +118,24 @@ Feature: LDAP And the command output contains the text "Clean up the user's remnants by" And invoking occ with "user:delete alice" Then the command output contains the text "The specified user was deleted" + + Scenario: Search only with group members - allowed + Given modify LDAP configuration + | ldapGroupFilter | cn=Orcharding | + | ldapGroupMemberAssocAttr | member | + | ldapBaseGroups | ou=OtherGroups,dc=nextcloud,dc=ci | + | ldapAttributesForUserSearch | employeeNumber | + | useMemberOfToDetectMembership | 1 | + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + And As an "alice" + When getting sharees for + # "5" is part of the employee number of some LDAP records + | search | 5 | + | itemType | file | + Then the OCS status code should be "200" + And the HTTP status code should be "200" + And "exact users" sharees returned is empty + And "users" sharees returned are + | Elisa | 0 | elisa | + And "exact groups" sharees returned is empty + From a6fe772deac485e237818e2bc57c513da5597a4c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 13 Jun 2019 16:49:39 +0200 Subject: [PATCH 2/2] fix inGroup check, thus make integration tests succeed there is not such strange return mode. Having invalid user ids caused this check to fail, and as side effect share limitation to groups to not work. Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Group_LDAP.php | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index cd4bd18cb44..0780e28f740 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -129,7 +129,6 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface, IGroupLD //usually, LDAP attributes are said to be case insensitive. But there are exceptions of course. $members = $this->_groupMembers($groupDN); - $members = array_keys($members); // uids are returned as keys if(!is_array($members) || count($members) === 0) { $this->access->connection->writeToCache($cacheKey, false); return false;