From ebb9f1d16a1b4f91c12cc4092218e8f4ff2a25a2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:31:28 +0100 Subject: [PATCH 01/17] Add a KnownUsers database with model Signed-off-by: Joas Schilling --- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- apps/dav/composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- apps/files/composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- apps/oauth2/composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../testing/composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../composer/composer/ClassLoader.php | 6 +- .../Version21000Date20210309185126.php | 69 ++++++++++++++++++ lib/composer/composer/ClassLoader.php | 6 +- lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + lib/private/KnownUser/KnownUser.php | 46 ++++++++++++ lib/private/KnownUser/KnownUserMapper.php | 72 +++++++++++++++++++ 31 files changed, 297 insertions(+), 52 deletions(-) create mode 100644 core/Migrations/Version21000Date20210309185126.php create mode 100644 lib/private/KnownUser/KnownUser.php create mode 100644 lib/private/KnownUser/KnownUserMapper.php diff --git a/apps/accessibility/composer/composer/ClassLoader.php b/apps/accessibility/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/accessibility/composer/composer/ClassLoader.php +++ b/apps/accessibility/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/admin_audit/composer/composer/ClassLoader.php b/apps/admin_audit/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/admin_audit/composer/composer/ClassLoader.php +++ b/apps/admin_audit/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/cloud_federation_api/composer/composer/ClassLoader.php b/apps/cloud_federation_api/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/cloud_federation_api/composer/composer/ClassLoader.php +++ b/apps/cloud_federation_api/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/comments/composer/composer/ClassLoader.php b/apps/comments/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/comments/composer/composer/ClassLoader.php +++ b/apps/comments/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/contactsinteraction/composer/composer/ClassLoader.php b/apps/contactsinteraction/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/contactsinteraction/composer/composer/ClassLoader.php +++ b/apps/contactsinteraction/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/dav/composer/composer/ClassLoader.php b/apps/dav/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/dav/composer/composer/ClassLoader.php +++ b/apps/dav/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/encryption/composer/composer/ClassLoader.php b/apps/encryption/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/encryption/composer/composer/ClassLoader.php +++ b/apps/encryption/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/federatedfilesharing/composer/composer/ClassLoader.php b/apps/federatedfilesharing/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/federatedfilesharing/composer/composer/ClassLoader.php +++ b/apps/federatedfilesharing/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/federation/composer/composer/ClassLoader.php b/apps/federation/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/federation/composer/composer/ClassLoader.php +++ b/apps/federation/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/files/composer/composer/ClassLoader.php b/apps/files/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/files/composer/composer/ClassLoader.php +++ b/apps/files/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/files_sharing/composer/composer/ClassLoader.php b/apps/files_sharing/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/files_sharing/composer/composer/ClassLoader.php +++ b/apps/files_sharing/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/files_trashbin/composer/composer/ClassLoader.php b/apps/files_trashbin/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/files_trashbin/composer/composer/ClassLoader.php +++ b/apps/files_trashbin/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/files_versions/composer/composer/ClassLoader.php b/apps/files_versions/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/files_versions/composer/composer/ClassLoader.php +++ b/apps/files_versions/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/lookup_server_connector/composer/composer/ClassLoader.php b/apps/lookup_server_connector/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/lookup_server_connector/composer/composer/ClassLoader.php +++ b/apps/lookup_server_connector/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/oauth2/composer/composer/ClassLoader.php b/apps/oauth2/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/oauth2/composer/composer/ClassLoader.php +++ b/apps/oauth2/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/provisioning_api/composer/composer/ClassLoader.php b/apps/provisioning_api/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/provisioning_api/composer/composer/ClassLoader.php +++ b/apps/provisioning_api/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/settings/composer/composer/ClassLoader.php b/apps/settings/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/settings/composer/composer/ClassLoader.php +++ b/apps/settings/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/sharebymail/composer/composer/ClassLoader.php b/apps/sharebymail/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/sharebymail/composer/composer/ClassLoader.php +++ b/apps/sharebymail/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/systemtags/composer/composer/ClassLoader.php b/apps/systemtags/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/systemtags/composer/composer/ClassLoader.php +++ b/apps/systemtags/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/testing/composer/composer/ClassLoader.php b/apps/testing/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/testing/composer/composer/ClassLoader.php +++ b/apps/testing/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/twofactor_backupcodes/composer/composer/ClassLoader.php b/apps/twofactor_backupcodes/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/twofactor_backupcodes/composer/composer/ClassLoader.php +++ b/apps/twofactor_backupcodes/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/updatenotification/composer/composer/ClassLoader.php b/apps/updatenotification/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/updatenotification/composer/composer/ClassLoader.php +++ b/apps/updatenotification/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/user_ldap/composer/composer/ClassLoader.php b/apps/user_ldap/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/user_ldap/composer/composer/ClassLoader.php +++ b/apps/user_ldap/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/user_status/composer/composer/ClassLoader.php b/apps/user_status/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/user_status/composer/composer/ClassLoader.php +++ b/apps/user_status/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/apps/workflowengine/composer/composer/ClassLoader.php b/apps/workflowengine/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/apps/workflowengine/composer/composer/ClassLoader.php +++ b/apps/workflowengine/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/core/Migrations/Version21000Date20210309185126.php b/core/Migrations/Version21000Date20210309185126.php new file mode 100644 index 0000000000000..675cda9e03fce --- /dev/null +++ b/core/Migrations/Version21000Date20210309185126.php @@ -0,0 +1,69 @@ + + * + * @author Joas Schilling + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OC\Core\Migrations; + +use Closure; +use Doctrine\DBAL\Types\Types; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version21000Date20210309185126 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('known_users')) { + $table = $schema->createTable('known_users'); + + // Auto increment id + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + ]); + + $table->addColumn('known_to', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + $table->addColumn('known_user', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + + $table->setPrimaryKey(['id']); + $table->addIndex(['known_to'], 'ku_known_to'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/ClassLoader.php b/lib/composer/composer/ClassLoader.php index 4d989a212c9f0..247294d66ee04 100644 --- a/lib/composer/composer/ClassLoader.php +++ b/lib/composer/composer/ClassLoader.php @@ -311,8 +311,10 @@ public function register($prepend = false) spl_autoload_register(array($this, 'loadClass'), true, $prepend); if (null === $this->vendorDir) { - //no-op - } elseif ($prepend) { + return; + } + + if ($prepend) { self::$registeredLoaders = array($this->vendorDir => $this) + self::$registeredLoaders; } else { unset(self::$registeredLoaders[$this->vendorDir]); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 49b138714a7d1..bfac84950182c 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -950,6 +950,7 @@ 'OC\\Core\\Migrations\\Version21000Date20201120141228' => $baseDir . '/core/Migrations/Version21000Date20201120141228.php', 'OC\\Core\\Migrations\\Version21000Date20201202095923' => $baseDir . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => $baseDir . '/core/Migrations/Version21000Date20210119195004.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', @@ -1168,6 +1169,8 @@ 'OC\\IntegrityCheck\\Helpers\\FileAccessHelper' => $baseDir . '/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFileByNameFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFileByNameFilterIterator.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', + 'OC\\KnownUser\\KnownUser' => $baseDir . '/lib/private/KnownUser/KnownUser.php', + 'OC\\KnownUser\\KnownUserMapper' => $baseDir . '/lib/private/KnownUser/KnownUserMapper.php', 'OC\\L10N\\Factory' => $baseDir . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => $baseDir . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => $baseDir . '/lib/private/L10N/L10NString.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 373db9144bd8f..349b84fdcf8cb 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -979,6 +979,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20201120141228' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201120141228.php', 'OC\\Core\\Migrations\\Version21000Date20201202095923' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20201202095923.php', 'OC\\Core\\Migrations\\Version21000Date20210119195004' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210119195004.php', + 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', @@ -1197,6 +1198,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\IntegrityCheck\\Helpers\\FileAccessHelper' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFileByNameFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFileByNameFilterIterator.php', 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', + 'OC\\KnownUser\\KnownUser' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUser.php', + 'OC\\KnownUser\\KnownUserMapper' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUserMapper.php', 'OC\\L10N\\Factory' => __DIR__ . '/../../..' . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => __DIR__ . '/../../..' . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => __DIR__ . '/../../..' . '/lib/private/L10N/L10NString.php', diff --git a/lib/private/KnownUser/KnownUser.php b/lib/private/KnownUser/KnownUser.php new file mode 100644 index 0000000000000..939c9199c7930 --- /dev/null +++ b/lib/private/KnownUser/KnownUser.php @@ -0,0 +1,46 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\KnownUser; + +use OCP\AppFramework\Db\Entity; + +/** + * @method void setKnownTo(string $knownTo) + * @method string getKnownTo() + * @method void setKnownUser(string $knownUser) + * @method string getKnownUser() + */ +class KnownUser extends Entity { + + /** @var string */ + protected $knownTo; + + /** @var string */ + protected $knownUser; + + public function __construct() { + $this->addType('knownTo', 'string'); + $this->addType('knownUser', 'string'); + } +} diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php new file mode 100644 index 0000000000000..86e13a2e2f076 --- /dev/null +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -0,0 +1,72 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\KnownUser; + +use OCP\AppFramework\Db\QBMapper; +use OCP\IDBConnection; + +/** + * @method KnownUser mapRowToEntity(array $row) + */ +class KnownUserMapper extends QBMapper { + + /** + * @param IDBConnection $db + */ + public function __construct(IDBConnection $db) { + parent::__construct($db, 'known_users', KnownUser::class); + } + + /** + * @param string $knownTo + * @return int Number of deleted entities + */ + public function deleteKnownTo(string $knownTo): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('known_to', $query->createNamedParameter($knownTo))); + + return (int) $query->execute(); + } + + /** + * @param string $knownUser + * @return int Number of deleted entities + */ + public function deleteKnownUser(string $knownUser): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('known_user', $query->createNamedParameter($knownUser))); + + return (int) $query->execute(); + } + + public function createKnownUserFromRow(array $row): KnownUser { + return $this->mapRowToEntity([ + 'id' => $row['s_id'], + 'known_to' => $row['known_to'], + 'known_user' => $row['known_user'], + ]); + } +} From 3feca65399d1d4d1183d6c052eae815d5b9cdc77 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:32:08 +0100 Subject: [PATCH 02/17] Store when a user knows another user based on phone number Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 34c0135485f6a..803bdf6d91acc 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -49,6 +49,8 @@ use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; +use OC\KnownUser\KnownUser; +use OC\KnownUser\KnownUserMapper; use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; @@ -89,6 +91,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; + /** @var KnownUserMapper */ + private $knownUserMapper; /** @var IEventDispatcher */ private $eventDispatcher; @@ -107,6 +111,7 @@ public function __construct(string $appName, FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, + KnownUserMapper $knownUserMapper, IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, @@ -125,6 +130,7 @@ public function __construct(string $appName, $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; + $this->knownUserMapper = $knownUserMapper; $this->eventDispatcher = $eventDispatcher; } @@ -264,9 +270,25 @@ public function searchByPhoneNumbers(string $location, array $search): DataRespo } $matches = []; + $knownUsers = []; foreach ($userMatches as $phone => $userId) { // Not using the ICloudIdManager as that would run a search for each contact to find the display name in the address book $matches[$normalizedNumberToKey[$phone]] = $userId . '@' . $cloudUrl; + $knownUsers[] = $userId; + } + + /** @var IUser $user */ + $user = $this->userSession->getUser(); + $knownTo = $user->getUID(); + + // Cleanup all previous entries and only allow new matches + $this->knownUserMapper->deleteKnownTo($knownTo); + + foreach ($knownUsers as $knownUser) { + $entity = new KnownUser(); + $entity->setKnownTo($knownTo); + $entity->setKnownUser($knownUser); + $this->knownUserMapper->insert($entity); } return new DataResponse($matches); From 19ccc992d5be9a3e493dda8c2c5f4c00ab7ba8bc Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:32:50 +0100 Subject: [PATCH 03/17] Delete matches when a user changes their phone number Signed-off-by: Joas Schilling --- apps/provisioning_api/lib/Controller/UsersController.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 803bdf6d91acc..70450756be72c 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -686,6 +686,10 @@ public function editUser(string $userId, string $key, string $value): DataRespon $userAccount[$key]['value'] = $value; try { $this->accountManager->updateUser($targetUser, $userAccount, true); + + if ($key === IAccountManager::PROPERTY_PHONE) { + $this->knownUserMapper->deleteKnownUser($targetUser->getUID()); + } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } From 5af22f84b38b5b1951d04e92fb65b6dde7d11fee Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 20:36:02 +0100 Subject: [PATCH 04/17] Delete matches when the user is being deleted Signed-off-by: Joas Schilling --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/AppInfo/Application.php | 4 ++ .../lib/Listener/UserDeletedListener.php | 54 +++++++++++++++++++ 4 files changed, 60 insertions(+) create mode 100644 apps/provisioning_api/lib/Listener/UserDeletedListener.php diff --git a/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index 0383fddbefda2..e94a97c194911 100644 --- a/apps/provisioning_api/composer/composer/autoload_classmap.php +++ b/apps/provisioning_api/composer/composer/autoload_classmap.php @@ -14,6 +14,7 @@ 'OCA\\Provisioning_API\\Controller\\GroupsController' => $baseDir . '/../lib/Controller/GroupsController.php', 'OCA\\Provisioning_API\\Controller\\UsersController' => $baseDir . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => $baseDir . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => $baseDir . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => $baseDir . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => $baseDir . '/../lib/Middleware/ProvisioningApiMiddleware.php', ); diff --git a/apps/provisioning_api/composer/composer/autoload_static.php b/apps/provisioning_api/composer/composer/autoload_static.php index 2c1682641a115..b982f203211ae 100644 --- a/apps/provisioning_api/composer/composer/autoload_static.php +++ b/apps/provisioning_api/composer/composer/autoload_static.php @@ -29,6 +29,7 @@ class ComposerStaticInitProvisioning_API 'OCA\\Provisioning_API\\Controller\\GroupsController' => __DIR__ . '/..' . '/../lib/Controller/GroupsController.php', 'OCA\\Provisioning_API\\Controller\\UsersController' => __DIR__ . '/..' . '/../lib/Controller/UsersController.php', 'OCA\\Provisioning_API\\FederatedShareProviderFactory' => __DIR__ . '/..' . '/../lib/FederatedShareProviderFactory.php', + 'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => __DIR__ . '/..' . '/../lib/Listener/UserDeletedListener.php', 'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => __DIR__ . '/..' . '/../lib/Middleware/Exceptions/NotSubAdminException.php', 'OCA\\Provisioning_API\\Middleware\\ProvisioningApiMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/ProvisioningApiMiddleware.php', ); diff --git a/apps/provisioning_api/lib/AppInfo/Application.php b/apps/provisioning_api/lib/AppInfo/Application.php index 863f8861d8ba7..7ec21c3329e13 100644 --- a/apps/provisioning_api/lib/AppInfo/Application.php +++ b/apps/provisioning_api/lib/AppInfo/Application.php @@ -29,6 +29,7 @@ namespace OCA\Provisioning_API\AppInfo; use OC\Group\Manager as GroupManager; +use OCA\Provisioning_API\Listener\UserDeletedListener; use OCA\Provisioning_API\Middleware\ProvisioningApiMiddleware; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\AppFramework\App; @@ -47,6 +48,7 @@ use OCP\Mail\IMailer; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Psr\Container\ContainerInterface; @@ -56,6 +58,8 @@ public function __construct(array $urlParams = []) { } public function register(IRegistrationContext $context): void { + $context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class); + $context->registerService(NewUserMailHelper::class, function (ContainerInterface $c) { return new NewUserMailHelper( $c->get(Defaults::class), diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php new file mode 100644 index 0000000000000..bcbf8cc85b604 --- /dev/null +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -0,0 +1,54 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Provisioning_API\Listener; + +use OC\KnownUser\KnownUserMapper; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\User\Events\UserDeletedEvent; + +class UserDeletedListener implements IEventListener { + + /** @var KnownUserMapper */ + private $knownUserMapper; + + public function __construct(KnownUserMapper $knownUserMapper) { + $this->knownUserMapper = $knownUserMapper; + } + + public function handle(Event $event): void { + if (!($event instanceof UserDeletedEvent)) { + // Unrelated + return; + } + + $user = $event->getUser(); + + // Delete all entries of this user + $this->knownUserMapper->deleteKnownTo($user->getUID()); + + // Delete all entries that other users know this user + $this->knownUserMapper->deleteKnownUser($user->getUID()); + } +} From 88855d88273cd413eaf882558ee49ec8ef03ade7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:22:59 +0100 Subject: [PATCH 05/17] Add a service to find out if a user knows another user Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 37 +++++------ .../lib/Listener/UserDeletedListener.php | 14 ++--- .../tests/Controller/UsersControllerTest.php | 23 +++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/KnownUser/KnownUserMapper.php | 13 ++++ lib/private/KnownUser/KnownUserService.php | 62 +++++++++++++++++++ 7 files changed, 121 insertions(+), 30 deletions(-) create mode 100644 lib/private/KnownUser/KnownUserService.php diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 70450756be72c..198d6657468aa 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -49,8 +49,7 @@ use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\HintException; -use OC\KnownUser\KnownUser; -use OC\KnownUser\KnownUserMapper; +use OC\KnownUser\KnownUserService; use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\Accounts\IAccountManager; @@ -91,8 +90,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; - /** @var KnownUserMapper */ - private $knownUserMapper; + /** @var KnownUserService */ + private $knownUserService; /** @var IEventDispatcher */ private $eventDispatcher; @@ -111,7 +110,7 @@ public function __construct(string $appName, FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, RemoteWipe $remoteWipe, - KnownUserMapper $knownUserMapper, + KnownUserService $knownUserService, IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, @@ -130,7 +129,7 @@ public function __construct(string $appName, $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; - $this->knownUserMapper = $knownUserMapper; + $this->knownUserService = $knownUserService; $this->eventDispatcher = $eventDispatcher; } @@ -236,6 +235,13 @@ public function searchByPhoneNumbers(string $location, array $search): DataRespo return new DataResponse([], Http::STATUS_BAD_REQUEST); } + /** @var IUser $user */ + $user = $this->userSession->getUser(); + $knownTo = $user->getUID(); + + // Cleanup all previous entries and only allow new matches + $this->knownUserService->deleteKnownTo($knownTo); + $normalizedNumberToKey = []; foreach ($search as $key => $phoneNumbers) { foreach ($phoneNumbers as $phone) { @@ -270,25 +276,10 @@ public function searchByPhoneNumbers(string $location, array $search): DataRespo } $matches = []; - $knownUsers = []; foreach ($userMatches as $phone => $userId) { // Not using the ICloudIdManager as that would run a search for each contact to find the display name in the address book $matches[$normalizedNumberToKey[$phone]] = $userId . '@' . $cloudUrl; - $knownUsers[] = $userId; - } - - /** @var IUser $user */ - $user = $this->userSession->getUser(); - $knownTo = $user->getUID(); - - // Cleanup all previous entries and only allow new matches - $this->knownUserMapper->deleteKnownTo($knownTo); - - foreach ($knownUsers as $knownUser) { - $entity = new KnownUser(); - $entity->setKnownTo($knownTo); - $entity->setKnownUser($knownUser); - $this->knownUserMapper->insert($entity); + $this->knownUserService->storeIsKnownToUser($knownTo, $userId); } return new DataResponse($matches); @@ -688,7 +679,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon $this->accountManager->updateUser($targetUser, $userAccount, true); if ($key === IAccountManager::PROPERTY_PHONE) { - $this->knownUserMapper->deleteKnownUser($targetUser->getUID()); + $this->knownUserService->deleteKnownUser($targetUser->getUID()); } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php index bcbf8cc85b604..f4fdb973080b3 100644 --- a/apps/provisioning_api/lib/Listener/UserDeletedListener.php +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -23,18 +23,18 @@ namespace OCA\Provisioning_API\Listener; -use OC\KnownUser\KnownUserMapper; +use OC\KnownUser\KnownUserService; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\User\Events\UserDeletedEvent; class UserDeletedListener implements IEventListener { - /** @var KnownUserMapper */ - private $knownUserMapper; + /** @var KnownUserService */ + private $service; - public function __construct(KnownUserMapper $knownUserMapper) { - $this->knownUserMapper = $knownUserMapper; + public function __construct(KnownUserService $service) { + $this->service = $service; } public function handle(Event $event): void { @@ -46,9 +46,9 @@ public function handle(Event $event): void { $user = $event->getUser(); // Delete all entries of this user - $this->knownUserMapper->deleteKnownTo($user->getUID()); + $this->service->deleteKnownTo($user->getUID()); // Delete all entries that other users know this user - $this->knownUserMapper->deleteKnownUser($user->getUID()); + $this->service->deleteKnownUser($user->getUID()); } } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 10f5a4841d471..39743579b7a21 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -44,6 +44,7 @@ use OC\Accounts\AccountManager; use OC\Authentication\Token\RemoteWipe; use OC\Group\Manager; +use OC\KnownUser\KnownUserService; use OC\SubAdmin; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Provisioning_API\Controller\UsersController; @@ -102,6 +103,8 @@ class UsersControllerTest extends TestCase { private $secureRandom; /** @var RemoteWipe|MockObject */ private $remoteWipe; + /** @var KnownUserService|MockObject */ + private $knownUserService; /** @var IEventDispatcher */ private $eventDispatcher; @@ -122,6 +125,7 @@ protected function setUp(): void { $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->remoteWipe = $this->createMock(RemoteWipe::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->api = $this->getMockBuilder(UsersController::class) @@ -141,6 +145,7 @@ protected function setUp(): void { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['fillStorageInfo']) @@ -405,6 +410,7 @@ public function testAddUserSuccessfulWithDisplayName() { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['editUser']) @@ -1399,6 +1405,13 @@ public function dataSearchByPhoneNumbers(): array { * @param array $expected */ public function testSearchByPhoneNumbers(string $location, array $search, int $status, ?array $searchUsers, ?array $userMatches, array $expected) { + $knownTo = 'knownTo'; + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn($knownTo); + $this->userSession->method('getUser') + ->willReturn($user); + if ($searchUsers === null) { $this->accountManager->expects($this->never()) ->method('searchUsers'); @@ -1407,6 +1420,14 @@ public function testSearchByPhoneNumbers(string $location, array $search, int $s ->method('searchUsers') ->with(IAccountManager::PROPERTY_PHONE, $searchUsers) ->willReturn($userMatches); + + $this->knownUserService->expects($this->once()) + ->method('deleteKnownTo') + ->with($knownTo); + + $this->knownUserService->expects($this->exactly(count($expected))) + ->method('storeIsKnownToUser') + ->with($knownTo, $this->anything()); } $this->urlGenerator->method('getAbsoluteURL') @@ -3228,6 +3249,7 @@ public function testGetCurrentUserLoggedIn() { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['getUserData']) @@ -3294,6 +3316,7 @@ public function testGetUser() { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->knownUserService, $this->eventDispatcher, ]) ->setMethods(['getUserData']) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index bfac84950182c..fe24e8170876d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1171,6 +1171,7 @@ 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\KnownUser\\KnownUser' => $baseDir . '/lib/private/KnownUser/KnownUser.php', 'OC\\KnownUser\\KnownUserMapper' => $baseDir . '/lib/private/KnownUser/KnownUserMapper.php', + 'OC\\KnownUser\\KnownUserService' => $baseDir . '/lib/private/KnownUser/KnownUserService.php', 'OC\\L10N\\Factory' => $baseDir . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => $baseDir . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => $baseDir . '/lib/private/L10N/L10NString.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 349b84fdcf8cb..1827ac55d4204 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1200,6 +1200,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\KnownUser\\KnownUser' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUser.php', 'OC\\KnownUser\\KnownUserMapper' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUserMapper.php', + 'OC\\KnownUser\\KnownUserService' => __DIR__ . '/../../..' . '/lib/private/KnownUser/KnownUserService.php', 'OC\\L10N\\Factory' => __DIR__ . '/../../..' . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => __DIR__ . '/../../..' . '/lib/private/L10N/L10N.php', 'OC\\L10N\\L10NString' => __DIR__ . '/../../..' . '/lib/private/L10N/L10NString.php', diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php index 86e13a2e2f076..1144e2fd21200 100644 --- a/lib/private/KnownUser/KnownUserMapper.php +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -62,6 +62,19 @@ public function deleteKnownUser(string $knownUser): int { return (int) $query->execute(); } + /** + * @param string $knownTo + * @return KnownUser[] + */ + public function getKnownTo(string $knownTo): array { + $query = $this->db->getQueryBuilder(); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->eq('known_to', $query->createNamedParameter($knownTo))); + + return $this->findEntities($query); + } + public function createKnownUserFromRow(array $row): KnownUser { return $this->mapRowToEntity([ 'id' => $row['s_id'], diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php new file mode 100644 index 0000000000000..3a97b967c3a98 --- /dev/null +++ b/lib/private/KnownUser/KnownUserService.php @@ -0,0 +1,62 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\KnownUser; + +class KnownUserService { + /** @var KnownUserMapper */ + protected $mapper; + /** @var array */ + protected $knownUsers = []; + + public function __construct(KnownUserMapper $mapper) { + $this->mapper = $mapper; + } + + public function deleteKnownTo(string $knownTo): int { + return $this->mapper->deleteKnownTo($knownTo); + } + + public function deleteKnownUser(string $knownUser): int { + return $this->mapper->deleteKnownUser($knownUser); + } + + public function storeIsKnownToUser(string $knownTo, string $knownUser): void { + $entity = new KnownUser(); + $entity->setKnownTo($knownTo); + $entity->setKnownUser($knownUser); + $this->mapper->insert($entity); + } + + public function isKnownToUser(string $knownTo, string $user): bool { + if (!isset($this->knownUsers[$knownTo])) { + $entities = $this->mapper->getKnownTo($knownTo); + $this->knownUsers[$knownTo] = []; + foreach ($entities as $entity) { + $this->knownUsers[$knownTo][$entity->getKnownUser()] = true; + } + } + + return isset($this->knownUsers[$knownTo][$user]); + } +} From 52d43bf3cc5d9743bb2949ad2fdd6b419b21f7b3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:46:42 +0100 Subject: [PATCH 06/17] Add a config setting to restrict autocompletion to phonebook matches Signed-off-by: Joas Schilling --- apps/settings/js/admin.js | 1 + apps/settings/lib/Settings/Admin/Sharing.php | 1 + .../templates/settings/admin/sharing.php | 12 +- .../tests/Settings/Admin/SharingTest.php | 221 ++++-------------- 4 files changed, 57 insertions(+), 178 deletions(-) diff --git a/apps/settings/js/admin.js b/apps/settings/js/admin.js index cffaefa3821b3..72b167d7e0d71 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -144,6 +144,7 @@ window.addEventListener('DOMContentLoaded', function(){ $('#shareapi_allow_share_dialog_user_enumeration').on('change', function() { $('#shareapi_restrict_user_enumeration_to_group_setting').toggleClass('hidden', !this.checked); + $('#shareapi_restrict_user_enumeration_to_phone_setting').toggleClass('hidden', !this.checked); }) $('#allowLinks').change(function() { diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 313a182501d94..19eed576cd719 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -73,6 +73,7 @@ public function getForm() { 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), 'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'), + 'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'), 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index 9f651ce6d6c20..b02a7d2764ca1 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -173,7 +173,17 @@ /> -
+
+

+ +

+ /> +

diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 52e83f8ba7fcf..5d0794170a07b 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -64,95 +64,28 @@ protected function setUp(): void { public function testGetFormWithoutExcludedGroups() { $this->config - ->expects($this->at(0)) ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups_list', '') - ->willReturn(''); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_group_sharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_public_upload', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(4)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_resharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(5)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(6)) - ->method('getAppValue') - ->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(7)) - ->method('getAppValue') - ->with('core', 'shareapi_enabled', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(8)) - ->method('getAppValue') - ->with('core', 'shareapi_default_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(9)) - ->method('getAppValue') - ->with('core', 'shareapi_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(10)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(11)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(12)) - ->method('getAppValue') - ->with('core', 'shareapi_public_link_disclaimertext', null) - ->willReturn('Lorem ipsum'); - $this->config - ->expects($this->at(13)) - ->method('getAppValue') - ->with('core', 'shareapi_enable_link_password_by_default', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(14)) - ->method('getAppValue') - ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) - ->willReturn(Constants::PERMISSION_ALL); - $this->config - ->expects($this->at(15)) - ->method('getAppValue') - ->with('core', 'shareapi_default_internal_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(16)) - ->method('getAppValue') - ->with('core', 'shareapi_internal_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(17)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_internal_expire_date', 'no') - ->willReturn('no'); + ->willReturnMap([ + ['core', 'shareapi_exclude_groups_list', '', ''], + ['core', 'shareapi_allow_group_sharing', 'yes', 'yes'], + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_public_upload', 'yes', 'yes'], + ['core', 'shareapi_allow_resharing', 'yes', 'yes'], + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_enabled', 'yes', 'yes'], + ['core', 'shareapi_default_expire_date', 'no', 'no'], + ['core', 'shareapi_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_expire_date', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_public_link_disclaimertext', null, 'Lorem ipsum'], + ['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'], + ['core', 'shareapi_default_permissions', Constants::PERMISSION_ALL, Constants::PERMISSION_ALL], + ['core', 'shareapi_default_internal_expire_date', 'no', 'no'], + ['core', 'shareapi_internal_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_internal_expire_date', 'no', 'no'], + ]); $expected = new TemplateResponse( 'settings', @@ -164,6 +97,7 @@ public function testGetFormWithoutExcludedGroups() { 'allowResharing' => 'yes', 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', + 'restrictUserEnumerationToPhone' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -188,96 +122,28 @@ public function testGetFormWithoutExcludedGroups() { public function testGetFormWithExcludedGroups() { $this->config - ->expects($this->at(0)) ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups_list', '') - ->willReturn('["NoSharers","OtherNoSharers"]'); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_group_sharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_links', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_public_upload', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(4)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_resharing', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(5)) - ->method('getAppValue') - ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(6)) - ->method('getAppValue') - ->with('core', 'shareapi_restrict_user_enumeration_to_group', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(7)) - ->method('getAppValue') - ->with('core', 'shareapi_enabled', 'yes') - ->willReturn('yes'); - $this->config - ->expects($this->at(8)) - ->method('getAppValue') - ->with('core', 'shareapi_default_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(9)) - ->method('getAppValue') - ->with('core', 'shareapi_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(10)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(11)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(12)) - ->method('getAppValue') - ->with('core', 'shareapi_public_link_disclaimertext', null) - ->willReturn('Lorem ipsum'); - $this->config - ->expects($this->at(13)) - ->method('getAppValue') - ->with('core', 'shareapi_enable_link_password_by_default', 'no') - ->willReturn('yes'); - $this->config - ->expects($this->at(14)) - ->method('getAppValue') - ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) - ->willReturn(Constants::PERMISSION_ALL); - $this->config - ->expects($this->at(15)) - ->method('getAppValue') - ->with('core', 'shareapi_default_internal_expire_date', 'no') - ->willReturn('no'); - $this->config - ->expects($this->at(16)) - ->method('getAppValue') - ->with('core', 'shareapi_internal_expire_after_n_days', '7') - ->willReturn('7'); - $this->config - ->expects($this->at(17)) - ->method('getAppValue') - ->with('core', 'shareapi_enforce_internal_expire_date', 'no') - ->willReturn('no'); - + ->willReturnMap([ + ['core', 'shareapi_exclude_groups_list', '', '["NoSharers","OtherNoSharers"]'], + ['core', 'shareapi_allow_group_sharing', 'yes', 'yes'], + ['core', 'shareapi_allow_links', 'yes', 'yes'], + ['core', 'shareapi_allow_public_upload', 'yes', 'yes'], + ['core', 'shareapi_allow_resharing', 'yes', 'yes'], + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_enabled', 'yes', 'yes'], + ['core', 'shareapi_default_expire_date', 'no', 'no'], + ['core', 'shareapi_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_expire_date', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'yes'], + ['core', 'shareapi_public_link_disclaimertext', null, 'Lorem ipsum'], + ['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'], + ['core', 'shareapi_default_permissions', Constants::PERMISSION_ALL, Constants::PERMISSION_ALL], + ['core', 'shareapi_default_internal_expire_date', 'no', 'no'], + ['core', 'shareapi_internal_expire_after_n_days', '7', '7'], + ['core', 'shareapi_enforce_internal_expire_date', 'no', 'no'], + ]); $expected = new TemplateResponse( 'settings', @@ -289,6 +155,7 @@ public function testGetFormWithExcludedGroups() { 'allowResharing' => 'yes', 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', + 'restrictUserEnumerationToPhone' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', From c7560ab3a934ccf0c6271dd223e2907bd7d57949 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 Mar 2021 21:48:48 +0100 Subject: [PATCH 07/17] Restrict autocompletion also based on the phonebook known users Signed-off-by: Joas Schilling --- apps/dav/appinfo/v1/caldav.php | 2 + apps/dav/appinfo/v1/carddav.php | 2 + apps/dav/lib/CardDAV/SystemAddressbook.php | 5 +- apps/dav/lib/Command/CreateCalendar.php | 2 + apps/dav/lib/Connector/Sabre/Principal.php | 88 ++-- apps/dav/lib/RootCollection.php | 2 + .../unit/CalDAV/AbstractCalDavBackend.php | 2 + .../tests/unit/CardDAV/CardDavBackendTest.php | 2 + .../unit/Connector/Sabre/PrincipalTest.php | 8 +- .../lib/AppInfo/Application.php | 2 + .../Collaborators/MailPlugin.php | 33 +- .../Collaborators/UserPlugin.php | 29 +- .../Contacts/ContactsMenu/ContactsStore.php | 106 +++-- lib/private/Share20/Manager.php | 5 + lib/public/Share/IManager.php | 8 + .../Collaborators/MailPluginTest.php | 14 +- .../Collaborators/UserPluginTest.php | 7 + .../ContactsMenu/ContactsStoreTest.php | 379 +++++++++++++++--- 18 files changed, 556 insertions(+), 140 deletions(-) diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index e04653ddea1ec..236d81f66f8fe 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -27,6 +27,7 @@ */ // Backends +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\Connector\LegacyDAVACL; use OCA\DAV\CalDAV\CalendarRoot; @@ -50,6 +51,7 @@ \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig(), 'principals/' ); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index dbab1ae9681a0..bb766bbaecaa2 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -27,6 +27,7 @@ */ // Backends +use OC\KnownUser\KnownUserService; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CardDAV\AddressBookRoot; use OCA\DAV\CardDAV\CardDavBackend; @@ -53,6 +54,7 @@ \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(\OCA\DAV\CalDAV\Proxy\ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig(), 'principals/' ); diff --git a/apps/dav/lib/CardDAV/SystemAddressbook.php b/apps/dav/lib/CardDAV/SystemAddressbook.php index c7190c81319f9..5b9521527111e 100644 --- a/apps/dav/lib/CardDAV/SystemAddressbook.php +++ b/apps/dav/lib/CardDAV/SystemAddressbook.php @@ -43,8 +43,9 @@ public function __construct(BackendInterface $carddavBackend, array $addressBook public function getChildren() { $shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; - $restrictShareEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; - if (!$shareEnumeration || ($shareEnumeration && $restrictShareEnumeration)) { + $shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + if (!$shareEnumeration || $shareEnumerationGroup || $shareEnumerationPhone) { return []; } diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php index 58c6a8c63fb03..1d543c71bc2ff 100644 --- a/apps/dav/lib/Command/CreateCalendar.php +++ b/apps/dav/lib/Command/CreateCalendar.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Command; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; @@ -86,6 +87,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int \OC::$server->getUserSession(), \OC::$server->getAppManager(), \OC::$server->query(ProxyMapper::class), + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig() ); $random = \OC::$server->getSecureRandom(); diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index c1b1dc1b2d161..94302a12b4918 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -36,6 +36,7 @@ namespace OCA\DAV\Connector\Sabre; +use OC\KnownUser\KnownUserService; use OCA\Circles\Exceptions\CircleDoesNotExistException; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Traits\PrincipalProxyTrait; @@ -82,27 +83,19 @@ class Principal implements BackendInterface { /** @var ProxyMapper */ private $proxyMapper; + /** @var KnownUserService */ + private $knownUserService; + /** @var IConfig */ private $config; - /** - * Principal constructor. - * - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param IShareManager $shareManager - * @param IUserSession $userSession - * @param IAppManager $appManager - * @param ProxyMapper $proxyMapper - * @param IConfig $config - * @param string $principalPrefix - */ public function __construct(IUserManager $userManager, IGroupManager $groupManager, IShareManager $shareManager, IUserSession $userSession, IAppManager $appManager, ProxyMapper $proxyMapper, + KnownUserService $knownUserService, IConfig $config, string $principalPrefix = 'principals/users/') { $this->userManager = $userManager; @@ -113,6 +106,7 @@ public function __construct(IUserManager $userManager, $this->principalPrefix = trim($principalPrefix, '/'); $this->hasGroups = $this->hasCircles = ($principalPrefix === 'principals/users/'); $this->proxyMapper = $proxyMapper; + $this->knownUserService = $knownUserService; $this->config = $config; } @@ -267,24 +261,24 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' } $allowEnumeration = $this->shareManager->allowEnumeration(); - $limitEnumeration = $this->shareManager->limitEnumerationToGroups(); + $limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups(); + $limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone(); // If sharing is restricted to group members only, // return only members that have groups in common $restrictGroups = false; + $currentUser = $this->userSession->getUser(); if ($this->shareManager->shareWithGroupMembersOnly()) { - $user = $this->userSession->getUser(); - if (!$user) { + if (!$currentUser instanceof IUser) { return []; } - $restrictGroups = $this->groupManager->getUserGroupIds($user); + $restrictGroups = $this->groupManager->getUserGroupIds($currentUser); } $currentUserGroups = []; - if ($limitEnumeration) { - $currentUser = $this->userSession->getUser(); - if ($currentUser) { + if ($limitEnumerationGroup) { + if ($currentUser instanceof IUser) { $currentUserGroups = $this->groupManager->getUserGroupIds($currentUser); } } @@ -302,14 +296,28 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' $users = \array_filter($users, static function (IUser $user) use ($value) { return $user->getEMailAddress() === $value; }); - } + } else { + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { + if ($user->getEMailAddress() === $value) { + return true; + } - if ($limitEnumeration) { - $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { - return !empty(array_intersect( - $this->groupManager->getUserGroupIds($user), - $currentUserGroups - )) || $user->getEMailAddress() === $value; + if ($limitEnumerationPhone + && $currentUser instanceof IUser + && $this->knownUserService->isKnownToUser($currentUser->getUID(), $user->getUID())) { + // Synced phonebook match + return true; + } + + if (!$limitEnumerationGroup) { + // No limitation on enumeration, all allowed + return true; + } + + return !empty($currentUserGroups) && !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )); }); } @@ -334,14 +342,28 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' $users = \array_filter($users, static function (IUser $user) use ($value) { return $user->getDisplayName() === $value; }); - } + } else { + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { + if ($user->getDisplayName() === $value) { + return true; + } + + if ($limitEnumerationPhone + && $currentUser instanceof IUser + && $this->knownUserService->isKnownToUser($currentUser->getUID(), $user->getUID())) { + // Synced phonebook match + return true; + } + + if (!$limitEnumerationGroup) { + // No limitation on enumeration, all allowed + return true; + } - if ($limitEnumeration) { - $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { - return !empty(array_intersect( - $this->groupManager->getUserGroupIds($user), - $currentUserGroups - )) || $user->getDisplayName() === $value; + return !empty($currentUserGroups) && !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )); }); } diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index 18874ecf74891..16a209a98f0c1 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -28,6 +28,7 @@ namespace OCA\DAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\CalendarRoot; @@ -70,6 +71,7 @@ public function __construct() { \OC::$server->getUserSession(), \OC::$server->getAppManager(), $proxyMapper, + \OC::$server->get(KnownUserService::class), \OC::$server->getConfig() ); $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $config); diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index 85efd0fd3699e..51ba8c1867afc 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Tests\unit\CalDAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; @@ -92,6 +93,7 @@ protected function setUp(): void { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(KnownUserService::class), $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index a8c7a78172467..60f46ce8fac2a 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -33,6 +33,7 @@ namespace OCA\DAV\Tests\unit\CardDAV; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\CardDAV\AddressBook; use OCA\DAV\CardDAV\CardDavBackend; @@ -139,6 +140,7 @@ protected function setUp(): void { $this->createMock(IUserSession::class), $this->createMock(IAppManager::class), $this->createMock(ProxyMapper::class), + $this->createMock(KnownUserService::class), $this->createMock(IConfig::class), ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 117707eaf2a3b..33c1ec1b5875a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -30,6 +30,7 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; +use OC\KnownUser\KnownUserService; use OC\User\User; use OCA\DAV\CalDAV\Proxy\Proxy; use OCA\DAV\CalDAV\Proxy\ProxyMapper; @@ -41,6 +42,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Share\IManager; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\PropPatch; use Test\TestCase; @@ -67,6 +69,8 @@ class PrincipalTest extends TestCase { /** @var ProxyMapper | \PHPUnit\Framework\MockObject\MockObject */ private $proxyMapper; + /** @var KnownUserService|MockObject */ + private $knownUserService; /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ private $config; @@ -77,6 +81,7 @@ protected function setUp(): void { $this->userSession = $this->createMock(IUserSession::class); $this->appManager = $this->createMock(IAppManager::class); $this->proxyMapper = $this->createMock(ProxyMapper::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->config = $this->createMock(IConfig::class); $this->connector = new \OCA\DAV\Connector\Sabre\Principal( @@ -86,6 +91,7 @@ protected function setUp(): void { $this->userSession, $this->appManager, $this->proxyMapper, + $this->knownUserService, $this->config ); parent::setUp(); @@ -442,7 +448,7 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul if ($groupsOnly) { $user = $this->createMock(IUser::class); - $this->userSession->expects($this->once()) + $this->userSession->expects($this->atLeastOnce()) ->method('getUser') ->willReturn($user); diff --git a/apps/files_versions/lib/AppInfo/Application.php b/apps/files_versions/lib/AppInfo/Application.php index afbd42ffc3f6e..e09ad7e90ae56 100644 --- a/apps/files_versions/lib/AppInfo/Application.php +++ b/apps/files_versions/lib/AppInfo/Application.php @@ -27,6 +27,7 @@ namespace OCA\Files_Versions\AppInfo; +use OC\KnownUser\KnownUserService; use OCA\DAV\CalDAV\Proxy\ProxyMapper; use OCA\DAV\Connector\Sabre\Principal; use OCA\Files\Event\LoadAdditionalScriptsEvent; @@ -72,6 +73,7 @@ public function register(IRegistrationContext $context): void { $server->getUserSession(), $server->getAppManager(), $server->get(ProxyMapper::class), + $server->get(KnownUserService::class), $server->getConfig() ); }); diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7bdd29afc4e7e..7da8cede6aa6d 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -27,6 +27,7 @@ namespace OC\Collaboration\Collaborators; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchPlugin; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; @@ -40,8 +41,14 @@ use OCP\Share\IShare; class MailPlugin implements ISearchPlugin { - protected $shareeEnumeration; + /* @var bool */ protected $shareWithGroupOnly; + /* @var bool */ + protected $shareeEnumeration; + /* @var bool */ + protected $shareeEnumerationInGroupOnly; + /* @var bool */ + protected $shareeEnumerationPhone; /** @var IManager */ private $contactsManager; @@ -52,20 +59,28 @@ class MailPlugin implements ISearchPlugin { /** @var IGroupManager */ private $groupManager; - + /** @var KnownUserService */ + private $knownUserService; /** @var IUserSession */ private $userSession; - public function __construct(IManager $contactsManager, ICloudIdManager $cloudIdManager, IConfig $config, IGroupManager $groupManager, IUserSession $userSession) { + public function __construct(IManager $contactsManager, + ICloudIdManager $cloudIdManager, + IConfig $config, + IGroupManager $groupManager, + KnownUserService $knownUserService, + IUserSession $userSession) { $this->contactsManager = $contactsManager; $this->cloudIdManager = $cloudIdManager; $this->config = $config; $this->groupManager = $groupManager; + $this->knownUserService = $knownUserService; $this->userSession = $userSession; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } /** @@ -77,6 +92,8 @@ public function __construct(IManager $contactsManager, ICloudIdManager $cloudIdM * @since 13.0.0 */ public function search($search, $limit, $offset, ISearchResult $searchResult) { + $currentUserId = $this->userSession->getUser()->getUID(); + $result = $userResults = ['wide' => [], 'exact' => []]; $userType = new SearchResultType('users'); $emailType = new SearchResultType('emails'); @@ -152,8 +169,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { continue; } - $addToWide = !$this->shareeEnumerationInGroupOnly; - if ($this->shareeEnumerationInGroupOnly) { + $addToWide = !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone); + if (!$addToWide && $this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $contact['UID'])) { + $addToWide = true; + } + + if (!$addToWide && $this->shareeEnumerationInGroupOnly) { $addToWide = false; $userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); foreach ($userGroups as $userGroup) { @@ -181,7 +202,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { } if ($exactEmailMatch - || isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch) { + || (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) { if ($exactEmailMatch) { $searchResult->markExactIdMatch($emailType); } diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index d832a42000cb2..5114ccd8eb54a 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -32,6 +32,7 @@ namespace OC\Collaboration\Collaborators; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchPlugin; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; @@ -46,8 +47,12 @@ class UserPlugin implements ISearchPlugin { /* @var bool */ protected $shareWithGroupOnly; + /* @var bool */ protected $shareeEnumeration; + /* @var bool */ protected $shareeEnumerationInGroupOnly; + /* @var bool */ + protected $shareeEnumerationPhone; /** @var IConfig */ private $config; @@ -57,33 +62,29 @@ class UserPlugin implements ISearchPlugin { private $userSession; /** @var IUserManager */ private $userManager; + /** @var KnownUserService */ + private $knownUserService; /** @var IUserStatusManager */ private $userStatusManager; - /** - * UserPlugin constructor. - * - * @param IConfig $config - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param IUserSession $userSession - * @param IUserStatusManager $userStatusManager - */ public function __construct(IConfig $config, IUserManager $userManager, IGroupManager $groupManager, IUserSession $userSession, + KnownUserService $knownUserService, IUserStatusManager $userStatusManager) { $this->config = $config; $this->groupManager = $groupManager; $this->userSession = $userSession; $this->userManager = $userManager; + $this->knownUserService = $knownUserService; $this->userStatusManager = $userStatusManager; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -91,6 +92,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { $users = []; $hasMoreResults = false; + $currentUserId = $this->userSession->getUser()->getUID(); $currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); if ($this->shareWithGroupOnly) { // Search in all the groups this user is part of @@ -168,11 +170,16 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { ]; } else { $addToWideResults = false; - if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) { + if ($this->shareeEnumeration && + !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone)) { + $addToWideResults = true; + } + + if ($this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $user->getUID())) { $addToWideResults = true; } - if ($this->shareeEnumerationInGroupOnly) { + if (!$addToWideResults && $this->shareeEnumerationInGroupOnly) { $commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user)); if (!empty($commonGroups)) { $addToWideResults = true; diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index e2bd7edc63d98..852765506c063 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -31,6 +31,7 @@ namespace OC\Contacts\ContactsMenu; +use OC\KnownUser\KnownUserService; use OCP\Contacts\ContactsMenu\IContactsStore; use OCP\Contacts\ContactsMenu\IEntry; use OCP\Contacts\IManager; @@ -53,20 +54,19 @@ class ContactsStore implements IContactsStore { /** @var IGroupManager */ private $groupManager; - /** - * @param IManager $contactsManager - * @param IConfig $config - * @param IUserManager $userManager - * @param IGroupManager $groupManager - */ + /** @var KnownUserService */ + private $knownUserService; + public function __construct(IManager $contactsManager, IConfig $config, IUserManager $userManager, - IGroupManager $groupManager) { + IGroupManager $groupManager, + KnownUserService $knownUserService) { $this->contactsManager = $contactsManager; $this->config = $config; $this->userManager = $userManager; $this->groupManager = $groupManager; + $this->knownUserService = $knownUserService; } /** @@ -103,7 +103,7 @@ public function getContacts(IUser $user, $filter, ?int $limit = null, ?int $offs } /** - * Filters the contacts. Applies 3 filters: + * Filters the contacts. Applied filters: * 1. filter the current user * 2. if the `shareapi_allow_share_dialog_user_enumeration` config option is * enabled it will filter all local users @@ -122,20 +122,21 @@ private function filterContacts(IUser $self, array $entries, $filter) { $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; - $restrictEnumeration = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; // whether to filter out local users $skipLocal = false; - // whether to filter out all users which doesn't have the same group as the current user - $ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes' || $restrictEnumeration; + // whether to filter out all users which don't have a common group as the current user + $ownGroupsOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $selfGroups = $this->groupManager->getUserGroupIds($self); if ($excludedGroups) { $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', ''); $decodedExcludeGroups = json_decode($excludedGroups, true); - $excludeGroupsList = ($decodedExcludeGroups !== null) ? $decodedExcludeGroups : []; + $excludeGroupsList = $decodedExcludeGroups ?? []; if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) { // a group of the current user is excluded -> filter all local users @@ -145,47 +146,76 @@ private function filterContacts(IUser $self, $selfUID = $self->getUID(); - return array_values(array_filter($entries, function (IEntry $entry) use ($self, $skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $filter) { - if ($skipLocal && $entry->getProperty('isLocalSystemBook') === true) { + return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $filter) { + if ($entry->getProperty('UID') === $selfUID) { return false; } - // Prevent enumerating local users - if ($disallowEnumeration && $entry->getProperty('isLocalSystemBook')) { - $filterUser = true; + if ($entry->getProperty('isLocalSystemBook')) { + if ($skipLocal) { + return false; + } + + $checkedCommonGroupAlready = false; + + // Prevent enumerating local users + if ($disallowEnumeration) { + $filterUser = true; - $mailAddresses = $entry->getEMailAddresses(); - foreach ($mailAddresses as $mailAddress) { - if ($mailAddress === $filter) { + $mailAddresses = $entry->getEMailAddresses(); + foreach ($mailAddresses as $mailAddress) { + if ($mailAddress === $filter) { + $filterUser = false; + break; + } + } + + if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { $filterUser = false; - break; } - } - if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { - $filterUser = false; - } + if ($filterUser) { + return false; + } + } elseif ($restrictEnumerationPhone || $restrictEnumerationGroup) { + $canEnumerate = false; + if ($restrictEnumerationPhone) { + $canEnumerate = $this->knownUserService->isKnownToUser($selfUID, $entry->getProperty('UID')); + } - if ($filterUser) { - return false; - } - } + if (!$canEnumerate && $restrictEnumerationGroup) { + $user = $this->userManager->get($entry->getProperty('UID')); - if ($ownGroupsOnly && $entry->getProperty('isLocalSystemBook') === true) { - $uid = $this->userManager->get($entry->getProperty('UID')); + if ($user === null) { + return false; + } - if ($uid === null) { - return false; + $contactGroups = $this->groupManager->getUserGroupIds($user); + $canEnumerate = !empty(array_intersect($contactGroups, $selfGroups)); + $checkedCommonGroupAlready = true; + } + + if (!$canEnumerate) { + return false; + } } - $contactGroups = $this->groupManager->getUserGroupIds($uid); - if (count(array_intersect($contactGroups, $selfGroups)) === 0) { - // no groups in common, so shouldn't see the contact - return false; + if ($ownGroupsOnly && !$checkedCommonGroupAlready) { + $user = $this->userManager->get($entry->getProperty('UID')); + + if ($user === null) { + return false; + } + + $contactGroups = $this->groupManager->getUserGroupIds($user); + if (empty(array_intersect($contactGroups, $selfGroups))) { + // no groups in common, so shouldn't see the contact + return false; + } } } - return $entry->getProperty('UID') !== $selfUID; + return true; })); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 9a2b413896b00..d7105873dfdeb 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1822,6 +1822,11 @@ public function limitEnumerationToGroups(): bool { $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; } + public function limitEnumerationToPhone(): bool { + return $this->allowEnumeration() && + $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + } + /** * Copied from \OC_Util::isSharingDisabledForUser * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 635ccc1483d68..0c8732b4b15ec 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -384,6 +384,14 @@ public function allowEnumeration(): bool; */ public function limitEnumerationToGroups(): bool; + /** + * Check if user enumeration is limited to the phonebook matches + * + * @return bool + * @since 21.0.1 + */ + public function limitEnumerationToPhone(): bool; + /** * Check if sharing is disabled for the given user * diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php index 141d4b680b70a..3128231a1088a 100644 --- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php @@ -26,6 +26,7 @@ use OC\Collaboration\Collaborators\MailPlugin; use OC\Collaboration\Collaborators\SearchResult; use OC\Federation\CloudIdManager; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\SearchResultType; use OCP\Contacts\IManager; use OCP\Federation\ICloudIdManager; @@ -55,6 +56,9 @@ class MailPluginTest extends TestCase { /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ protected $groupManager; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + protected $knownUserService; + /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ protected $userSession; @@ -64,6 +68,7 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->contactsManager = $this->createMock(IManager::class); $this->groupManager = $this->createMock(IGroupManager::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->userSession = $this->createMock(IUserSession::class); $this->cloudIdManager = new CloudIdManager($this->contactsManager); @@ -71,7 +76,14 @@ protected function setUp(): void { } public function instantiatePlugin() { - $this->plugin = new MailPlugin($this->contactsManager, $this->cloudIdManager, $this->config, $this->groupManager, $this->userSession); + $this->plugin = new MailPlugin( + $this->contactsManager, + $this->cloudIdManager, + $this->config, + $this->groupManager, + $this->knownUserService, + $this->userSession + ); } /** diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index 2806540d00e2b..f2e0e7e274b6a 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -25,6 +25,7 @@ use OC\Collaboration\Collaborators\SearchResult; use OC\Collaboration\Collaborators\UserPlugin; +use OC\KnownUser\KnownUserService; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\IConfig; use OCP\IGroup; @@ -49,6 +50,9 @@ class UserPluginTest extends TestCase { /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ protected $session; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + protected $knownUserService; + /** @var IUserStatusManager|\PHPUnit\Framework\MockObject\MockObject */ protected $userStatusManager; @@ -78,6 +82,8 @@ protected function setUp(): void { $this->session = $this->createMock(IUserSession::class); + $this->knownUserService = $this->createMock(KnownUserService::class); + $this->userStatusManager = $this->createMock(IUserStatusManager::class); $this->searchResult = new SearchResult(); @@ -93,6 +99,7 @@ public function instantiatePlugin() { $this->userManager, $this->groupManager, $this->session, + $this->knownUserService, $this->userStatusManager ); } diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index acfe83ac5585d..ad83178096e9f 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -26,11 +26,13 @@ namespace Tests\Contacts\ContactsMenu; use OC\Contacts\ContactsMenu\ContactsStore; +use OC\KnownUser\KnownUserService; use OCP\Contacts\IManager; use OCP\IConfig; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ContactsStoreTest extends TestCase { @@ -44,6 +46,8 @@ class ContactsStoreTest extends TestCase { private $groupManager; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var KnownUserService|MockObject */ + private $knownUserService; protected function setUp(): void { parent::setUp(); @@ -52,7 +56,14 @@ protected function setUp(): void { $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->config = $this->createMock(IConfig::class); - $this->contactsStore = new ContactsStore($this->contactsManager, $this->config, $this->userManager, $this->groupManager); + $this->knownUserService = $this->createMock(KnownUserService::class); + $this->contactsStore = new ContactsStore( + $this->contactsManager, + $this->config, + $this->userManager, + $this->groupManager, + $this->knownUserService + ); } public function testGetContactsWithoutFilter() { @@ -171,29 +182,16 @@ public function testGetContactsWithoutAvatarURI() { } public function testGetContactsWhenUserIsInExcludeGroups() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); - - $this->config->expects($this->at(1)) + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('no'); - - $this->config->expects($this->at(2)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('yes'); - - $this->config->expects($this->at(3)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('yes'); - - $this->config->expects($this->at(4)) - ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups_list'), $this->equalTo('')) - ->willReturn('["group1", "group5", "group6"]'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'yes'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ['core', 'shareapi_exclude_groups_list', '', '["group1", "group5", "group6"]'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -228,22 +226,94 @@ public function testGetContactsWhenUserIsInExcludeGroups() { } public function testGetContactsOnlyShareIfInTheSameGroup() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); - $this->config->expects($this->at(1)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('no'); + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); - $this->config->expects($this->at(2)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('no'); + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + $user2 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(1)) + ->method('get') + ->with('user2') + ->willReturn($user2); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->with($this->equalTo($user2)) + ->willReturn(['group2', 'group3']); + $user3 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(2)) + ->method('get') + ->with('user3') + ->willReturn($user3); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->with($this->equalTo($user3)) + ->willReturn(['group8', 'group9']); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } - $this->config->expects($this->at(3)) + public function testGetContactsOnlyEnumerateIfInTheSameGroup() { + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('yes'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -313,23 +383,229 @@ public function testGetContactsOnlyShareIfInTheSameGroup() { $this->assertEquals('contact', $entries[2]->getProperty('UID')); } - public function testGetContactsOnlyEnumerateIfInTheSameGroup() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('yes'); + public function testGetContactsOnlyEnumerateIfPhoneBookMatch() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'no'], + ]); - $this->config->expects($this->at(1)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_restrict_user_enumeration_to_group'), $this->equalTo('no')) - ->willReturn('yes'); + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); - $this->config->expects($this->at(2)) ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no')) - ->willReturn('no'); + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', true], + ['user001', 'user2', true], + ['user001', 'user3', false], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } - $this->config->expects($this->at(3)) + public function testGetContactsOnlyEnumerateIfPhoneBookMatchWithOwnGroupsOnly() { + $this->config ->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no')) - ->willReturn('no'); + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + $user2 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(1)) + ->method('get') + ->with('user2') + ->willReturn($user2); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->with($this->equalTo($user2)) + ->willReturn(['group2', 'group3']); + $user3 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(2)) + ->method('get') + ->with('user3') + ->willReturn($user3); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->with($this->equalTo($user3)) + ->willReturn(['group8', 'group9']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', true], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(3, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('contact', $entries[2]->getProperty('UID')); + } + + public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroup() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'no'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ + $currentUser = $this->createMock(IUser::class); + $currentUser->expects($this->once()) + ->method('getUID') + ->willReturn('user001'); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->with($this->equalTo($currentUser)) + ->willReturn(['group1', 'group2', 'group3']); + + $user1 = $this->createMock(IUser::class); + $this->userManager->expects($this->at(0)) + ->method('get') + ->with('user1') + ->willReturn($user1); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->with($this->equalTo($user1)) + ->willReturn(['group1']); + + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', false], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) + ->willReturn([ + [ + 'UID' => 'user1', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user2', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'user3', + 'isLocalSystemBook' => true + ], + [ + 'UID' => 'contact', + ], + ]); + + $entries = $this->contactsStore->getContacts($currentUser, ''); + + $this->assertCount(4, $entries); + $this->assertEquals('user1', $entries[0]->getProperty('UID')); + $this->assertEquals('user2', $entries[1]->getProperty('UID')); + $this->assertEquals('user3', $entries[2]->getProperty('UID')); + $this->assertEquals('contact', $entries[3]->getProperty('UID')); + } + + public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnly() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'], + ['core', 'shareapi_exclude_groups', 'no', 'no'], + ['core', 'shareapi_only_share_with_group_members', 'no', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $currentUser */ $currentUser = $this->createMock(IUser::class); @@ -370,6 +646,13 @@ public function testGetContactsOnlyEnumerateIfInTheSameGroup() { ->with($this->equalTo($user3)) ->willReturn(['group8', 'group9']); + $this->knownUserService->method('isKnownToUser') + ->willReturnMap([ + ['user001', 'user1', false], + ['user001', 'user2', true], + ['user001', 'user3', true], + ]); + $this->contactsManager->expects($this->once()) ->method('search') ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL'])) From 30610aa61567933f193fc19ee767f2fadc9c9ace Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 11:54:43 +0100 Subject: [PATCH 08/17] Add integration tests for autocomplete/get (similar to sharees API) Signed-off-by: Joas Schilling --- .../autocomplete.feature | 175 ++++++++++++++++++ build/integration/config/behat.yml | 10 + .../features/bootstrap/BasicStructure.php | 34 ++++ .../bootstrap/CollaborationContext.php | 71 +++++++ 4 files changed, 290 insertions(+) create mode 100644 build/integration/collaboration_features/autocomplete.feature create mode 100644 build/integration/features/bootstrap/CollaborationContext.php diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature new file mode 100644 index 0000000000000..0ca8ebbc10054 --- /dev/null +++ b/build/integration/collaboration_features/autocomplete.feature @@ -0,0 +1,175 @@ +Feature: autocomplete + Background: + Given using api version "2" + And group "commongroup" exists + And user "admin" belongs to group "commongroup" + And user "autocomplete" exists + And user "autocomplete2" exists + And user "autocomplete2" belongs to group "commongroup" + + Scenario: getting autocomplete + Given As an "admin" + When get autocomplete for "auto" + | id | source | + | autocomplete | users | + | autocomplete2 | users | + + + Scenario: getting autocomplete without enumeration + Given As an "admin" + When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | + | autocomplete | users | + + + Scenario: getting autocomplete with limited enumeration by group + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" + Then get autocomplete for "auto" + | id | source | + | autocomplete2 | users | + Then get autocomplete for "autocomplete" + | id | source | + | autocomplete | users | + | autocomplete2 | users | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | + + + Scenario: getting autocomplete with limited enumeration by phone + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + Then get autocomplete for "auto" + | id | source | + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + Given As an "admin" + Then get autocomplete for "auto" + | id | source | + + # admin populates they have the phone number + When search users by phone for region "DE" with + | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | autocomplete | users | + + + Scenario: getting autocomplete with limited enumeration by group or phone + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" + And parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + # admin populates they have the phone number + Given As an "admin" + When search users by phone for region "DE" with + | random-string1 | 0711 / 252 428-90 | + + Then get autocomplete for "auto" + | id | source | + | autocomplete | users | + | autocomplete2 | users | + + + Scenario: getting autocomplete with limited enumeration but sharing is group restricted + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" + And parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + # admin populates they have the phone number + Given As an "admin" + When search users by phone for region "DE" with + | random-string1 | 0711 / 252 428-90 | + + Then get autocomplete for "auto" + | id | source | + | autocomplete | users | + | autocomplete2 | users | + When parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + Then get autocomplete for "auto" + | id | source | + | autocomplete2 | users | + + + Scenario: getting autocomplete with limited enumeration by phone but user changes it + Given As an "admin" + When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" + Then get autocomplete for "auto" + | id | source | + + # autocomplete stores their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-90 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + Given As an "admin" + Then get autocomplete for "auto" + | id | source | + + # admin populates they have the phone number + When search users by phone for region "DE" with + | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | autocomplete | users | + + # autocomplete changes their phone number + Given As an "autocomplete" + And sending "PUT" to "/cloud/users/autocomplete" with + | key | phone | + | value | +49 711 / 25 24 28-91 | + And the HTTP status code should be "200" + And the OCS status code should be "200" + + Given As an "admin" + Then get autocomplete for "auto" + | id | source | + + # admin populates they have the new phone number + When search users by phone for region "DE" with + | random-string1 | 0711 / 252 428-91 | + Then get autocomplete for "auto" + | id | source | + | autocomplete | users | + + + Scenario: getting autocomplete without enumeration and sharing is group restricted + Given As an "admin" + When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" + And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" + + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index 79ffe58f6b682..0e577f5925e69 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -45,6 +45,16 @@ default: - admin - admin regular_user_password: 123456 + collaboration: + paths: + - "%paths.base%/../collaboration_features" + contexts: + - CollaborationContext: + baseUrl: http://localhost:8080/ocs/ + admin: + - admin + - admin + regular_user_password: 123456 sharees: paths: - "%paths.base%/../sharees_features" diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 5b01e80707d9e..cc5ac2e14b6e9 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -202,6 +202,40 @@ public function sendingToWith($verb, $url, $body) { } } + /** + * @param string $verb + * @param string $url + * @param TableNode|array|null $body + * @param array $headers + */ + protected function sendRequestForJSON(string $verb, string $url, $body = null, array $headers = []): void { + $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url; + $client = new Client(); + $options = []; + if ($this->currentUser === 'admin') { + $options['auth'] = ['admin', 'admin']; + } elseif (strpos($this->currentUser, 'guest') !== 0) { + $options['auth'] = [$this->currentUser, self::TEST_PASSWORD]; + } + if ($body instanceof TableNode) { + $fd = $body->getRowsHash(); + $options['form_params'] = $fd; + } elseif (is_array($body)) { + $options['form_params'] = $body; + } + + $options['headers'] = array_merge($headers, [ + 'OCS-ApiRequest' => 'true', + 'Accept' => 'application/json', + ]); + + try { + $this->response = $client->{$verb}($fullUrl, $options); + } catch (ClientException $ex) { + $this->response = $ex->getResponse(); + } + } + /** * @When /^sending "([^"]*)" with exact url to "([^"]*)"$/ * @param string $verb diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php new file mode 100644 index 0000000000000..8207267bf4d6e --- /dev/null +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -0,0 +1,71 @@ + + * + * @author Joas Schilling + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +use Behat\Behat\Context\Context; +use Behat\Gherkin\Node\TableNode; +use PHPUnit\Framework\Assert; + +require __DIR__ . '/../../vendor/autoload.php'; + +class CollaborationContext implements Context { + use Provisioning; + use AppConfiguration; + + /** + * @Then /^get autocomplete for "([^"]*)"$/ + * @param TableNode|null $formData + */ + public function getAutocomplete(string $search, TableNode $formData): void { + $query = $search === 'null' ? null : $search; + + $this->sendRequestForJSON('GET', '/core/autocomplete/get?itemType=files&itemId=123&search=' . $query, [ + 'itemType' => 'files', + 'itemId' => '123', + 'search' => $query, + ]); + $this->theHTTPStatusCodeShouldBe(200); + + $data = json_decode($this->response->getBody()->getContents(), true); + $suggestions = $data['ocs']['data']; + + Assert::assertCount(count($formData->getHash()), $suggestions, 'Suggestion count does not match'); + Assert::assertEquals($formData->getHash(), array_map(static function ($suggestion, $expected) { + $data = []; + if (isset($expected['id'])) { + $data['id'] = $suggestion['id']; + } + if (isset($expected['source'])) { + $data['source'] = $suggestion['source']; + } + return $data; + }, $suggestions, $formData->getHash())); + } + + protected function resetAppConfigs(): void { + $this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration'); + $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group'); + $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone'); + $this->deleteServerConfig('core', 'shareapi_only_share_with_group_members'); + } +} From 8069c52a85e9b11836225d542b449fff3906e765 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 15:17:36 +0100 Subject: [PATCH 09/17] Add a hint that the settings are OR based Signed-off-by: Joas Schilling --- apps/settings/js/admin.js | 1 + apps/settings/templates/settings/admin/sharing.php | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/apps/settings/js/admin.js b/apps/settings/js/admin.js index 72b167d7e0d71..ba6b480c79d30 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -145,6 +145,7 @@ window.addEventListener('DOMContentLoaded', function(){ $('#shareapi_allow_share_dialog_user_enumeration').on('change', function() { $('#shareapi_restrict_user_enumeration_to_group_setting').toggleClass('hidden', !this.checked); $('#shareapi_restrict_user_enumeration_to_phone_setting').toggleClass('hidden', !this.checked); + $('#shareapi_restrict_user_enumeration_combinewarning_setting').toggleClass('hidden', !this.checked); }) $('#allowLinks').change(function() { diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index b02a7d2764ca1..a72bf0bd590c5 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -185,6 +185,11 @@ } ?> />

+

+ t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>
+

Date: Wed, 10 Mar 2021 17:18:44 +0100 Subject: [PATCH 10/17] Add a setting to restrict returning a full match unless in phonebook or same group Signed-off-by: Joas Schilling --- apps/dav/lib/Connector/Sabre/Principal.php | 36 ++++--- .../unit/Connector/Sabre/PrincipalTest.php | 51 ++++++++++ apps/settings/lib/Settings/Admin/Sharing.php | 1 + .../templates/settings/admin/sharing.php | 11 ++- .../tests/Settings/Admin/SharingTest.php | 4 + .../autocomplete.feature | 36 +++++++ .../bootstrap/CollaborationContext.php | 1 + .../Collaborators/MailPlugin.php | 5 +- .../Collaborators/UserPlugin.php | 6 +- .../Contacts/ContactsMenu/ContactsStore.php | 7 +- lib/private/Share20/Manager.php | 4 + lib/public/Share/IManager.php | 8 ++ .../ContactsMenu/ContactsStoreTest.php | 93 ++++++++++++++++++- 13 files changed, 243 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 94302a12b4918..326338310d394 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -263,6 +263,7 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' $allowEnumeration = $this->shareManager->allowEnumeration(); $limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups(); $limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone(); + $allowEnumerationFullMatch = $this->shareManager->allowEnumerationFullMatch(); // If sharing is restricted to group members only, // return only members that have groups in common @@ -290,15 +291,19 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' foreach ($searchProperties as $prop => $value) { switch ($prop) { case '{http://sabredav.org/ns}email-address': - $users = $this->userManager->getByEmail($value); - if (!$allowEnumeration) { - $users = \array_filter($users, static function (IUser $user) use ($value) { - return $user->getEMailAddress() === $value; - }); + if ($allowEnumerationFullMatch) { + $users = $this->userManager->getByEmail($value); + $users = \array_filter($users, static function (IUser $user) use ($value) { + return $user->getEMailAddress() === $value; + }); + } else { + $users = []; + } } else { - $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { - if ($user->getEMailAddress() === $value) { + $users = $this->userManager->getByEmail($value); + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) { + if ($allowEnumerationFullMatch && $user->getEMailAddress() === $value) { return true; } @@ -336,15 +341,20 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof' break; case '{DAV:}displayname': - $users = $this->userManager->searchDisplayName($value, $searchLimit); if (!$allowEnumeration) { - $users = \array_filter($users, static function (IUser $user) use ($value) { - return $user->getDisplayName() === $value; - }); + if ($allowEnumerationFullMatch) { + $users = $this->userManager->searchDisplayName($value, $searchLimit); + $users = \array_filter($users, static function (IUser $user) use ($value) { + return $user->getDisplayName() === $value; + }); + } else { + $users = []; + } } else { - $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) { - if ($user->getDisplayName() === $value) { + $users = $this->userManager->searchDisplayName($value, $searchLimit); + $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) { + if ($allowEnumerationFullMatch && $user->getDisplayName() === $value) { return true; } diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 33c1ec1b5875a..c9e3d44bf8855 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -570,6 +570,10 @@ public function testSearchPrincipalWithEnumerationDisabledDisplayname() { ->method('shareWithGroupMembersOnly') ->willReturn(false); + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(true); + $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); @@ -592,6 +596,27 @@ public function testSearchPrincipalWithEnumerationDisabledDisplayname() { ['{DAV:}displayname' => 'User 2'])); } + public function testSearchPrincipalWithEnumerationDisabledDisplaynameOnFullMatch() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('allowEnumeration') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(false); + + $this->assertEquals([], $this->connector->searchPrincipals('principals/users', + ['{DAV:}displayname' => 'User 2'])); + } + public function testSearchPrincipalWithEnumerationDisabledEmail() { $this->shareManager->expects($this->once()) ->method('shareAPIEnabled') @@ -605,6 +630,10 @@ public function testSearchPrincipalWithEnumerationDisabledEmail() { ->method('shareWithGroupMembersOnly') ->willReturn(false); + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(true); + $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); @@ -627,6 +656,28 @@ public function testSearchPrincipalWithEnumerationDisabledEmail() { ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); } + public function testSearchPrincipalWithEnumerationDisabledEmailOnFullMatch() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('allowEnumeration') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->willReturn(false); + + $this->shareManager->expects($this->once()) + ->method('allowEnumerationFullMatch') + ->willReturn(false); + + + $this->assertEquals([], $this->connector->searchPrincipals('principals/users', + ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); + } + public function testSearchPrincipalWithEnumerationLimitedDisplayname() { $this->shareManager->expects($this->at(0)) ->method('shareAPIEnabled') diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 19eed576cd719..6285ef399a895 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -74,6 +74,7 @@ public function getForm() { 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), 'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'), 'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'), + 'restrictUserEnumerationFullMatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes'), 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index a72bf0bd590c5..d7c24943b2491 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -163,7 +163,7 @@ /> -
+

t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>

+

+ /> +
+

'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -132,6 +134,7 @@ public function testGetFormWithExcludedGroups() { ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], ['core', 'shareapi_enabled', 'yes', 'yes'], ['core', 'shareapi_default_expire_date', 'no', 'no'], ['core', 'shareapi_expire_after_n_days', '7', '7'], @@ -156,6 +159,7 @@ public function testGetFormWithExcludedGroups() { 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature index 0ca8ebbc10054..5e294709d7fa0 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -3,6 +3,7 @@ Feature: autocomplete Given using api version "2" And group "commongroup" exists And user "admin" belongs to group "commongroup" + And user "auto" exists And user "autocomplete" exists And user "autocomplete2" exists And user "autocomplete2" belongs to group "commongroup" @@ -20,9 +21,15 @@ Feature: autocomplete When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | + | auto | users | Then get autocomplete for "autocomplete" | id | source | | autocomplete | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | Scenario: getting autocomplete with limited enumeration by group @@ -30,6 +37,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete2 | users | Then get autocomplete for "autocomplete" | id | source | @@ -38,6 +46,13 @@ Feature: autocomplete Then get autocomplete for "autocomplete2" | id | source | | autocomplete2 | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "autocomplete" + | id | source | + | autocomplete2 | users | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | Scenario: getting autocomplete with limited enumeration by phone @@ -45,6 +60,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -57,10 +73,17 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -83,6 +106,13 @@ Feature: autocomplete When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + | autocomplete2 | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -108,6 +138,7 @@ Feature: autocomplete Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | | autocomplete2 | users | When parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" @@ -121,6 +152,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -133,12 +165,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | # autocomplete changes their phone number @@ -152,12 +186,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the new phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-91 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php index 8207267bf4d6e..cdba167e6775a 100644 --- a/build/integration/features/bootstrap/CollaborationContext.php +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -66,6 +66,7 @@ protected function resetAppConfigs(): void { $this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone'); + $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match'); $this->deleteServerConfig('core', 'shareapi_only_share_with_group_members'); } } diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7da8cede6aa6d..240e16374d54c 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -49,6 +49,8 @@ class MailPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IManager */ private $contactsManager; @@ -81,6 +83,7 @@ public function __construct(IManager $contactsManager, $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } /** @@ -137,7 +140,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { continue; } } - if ($exactEmailMatch) { + if ($exactEmailMatch && $this->shareeEnumerationFullMatch) { try { $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0]); } catch (\InvalidArgumentException $e) { diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 5114ccd8eb54a..06a8c6f0efd47 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -53,6 +53,8 @@ class UserPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IConfig */ private $config; @@ -85,6 +87,7 @@ public function __construct(IConfig $config, $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -150,6 +153,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { if ( + $this->shareeEnumerationFullMatch && $lowerSearch !== '' && (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch || strtolower($userEmail) === $lowerSearch) @@ -202,7 +206,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { } } - if ($offset === 0 && !$foundUserById) { + if ($this->shareeEnumerationFullMatch && $offset === 0 && !$foundUserById) { // On page one we try if the search result has a direct hit on the // user id and if so, we add that to the exact match list $user = $this->userManager->get($search); diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index 852765506c063..e0e0bf832b330 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -124,6 +124,7 @@ private function filterContacts(IUser $self, $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; $restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $allowEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; // whether to filter out local users @@ -146,7 +147,7 @@ private function filterContacts(IUser $self, $selfUID = $self->getUID(); - return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $filter) { + return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $allowEnumerationFullMatch, $filter) { if ($entry->getProperty('UID') === $selfUID) { return false; } @@ -160,6 +161,10 @@ private function filterContacts(IUser $self, // Prevent enumerating local users if ($disallowEnumeration) { + if (!$allowEnumerationFullMatch) { + return false; + } + $filterUser = true; $mailAddresses = $entry->getEMailAddresses(); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index d7105873dfdeb..d7e1d0535199f 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1827,6 +1827,10 @@ public function limitEnumerationToPhone(): bool { $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } + public function allowEnumerationFullMatch(): bool { + return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; + } + /** * Copied from \OC_Util::isSharingDisabledForUser * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 0c8732b4b15ec..606e64299181c 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -392,6 +392,14 @@ public function limitEnumerationToGroups(): bool; */ public function limitEnumerationToPhone(): bool; + /** + * Check if user enumeration is allowed to return on full match + * + * @return bool + * @since 21.0.1 + */ + public function allowEnumerationFullMatch(): bool; + /** * Check if sharing is disabled for the given user * diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index ad83178096e9f..ad201d86a2ad0 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -683,9 +683,12 @@ public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnl } public function testGetContactsWithFilter() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('no'); + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); @@ -766,6 +769,90 @@ public function testGetContactsWithFilter() { ], $entry[0]->getEMailAddresses()); } + public function testGetContactsWithFilterWithoutFullMatch() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'no'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ + $user = $this->createMock(IUser::class); + $this->contactsManager->expects($this->any()) + ->method('search') + ->willReturn([ + [ + 'UID' => 'a567', + 'FN' => 'Darren Roner', + 'EMAIL' => [ + 'darren@roner.au', + ], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'john', + 'FN' => 'John Doe', + 'EMAIL' => [ + 'john@example.com', + ], + 'isLocalSystemBook' => true, + ], + [ + 'FN' => 'Anne D', + 'EMAIL' => [ + 'anne@example.com', + ], + 'isLocalSystemBook' => false, + ], + ]); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('user123'); + + // Complete match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a567'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a56'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Complete match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.com'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.co'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Match on FN should not match + $entry = $this->contactsStore->getContacts($user, 'Darren Roner'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Don't filter users in local addressbook + $entry = $this->contactsStore->getContacts($user, 'Anne D'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + } + public function testFindOneUser() { $this->config->expects($this->at(0))->method('getAppValue') ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) From 5afaf3d06ccea7684a7be459f40d3d3ce0369e73 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 17:51:28 +0100 Subject: [PATCH 11/17] Fix CS hopefully Signed-off-by: Joas Schilling --- apps/settings/templates/settings/admin/sharing.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index d7c24943b2491..65e84d29120a9 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -191,12 +191,12 @@ t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?>

+ p('hidden'); +}?>"> /> + print_unescaped('checked="checked"'); +} ?> />

From 72fb176ec999426048a7119862f4bf1ef8d2c34f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 18:55:01 +0100 Subject: [PATCH 12/17] Change label also in the acceptance tests Signed-off-by: Joas Schilling --- tests/acceptance/features/bootstrap/SettingsContext.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/features/bootstrap/SettingsContext.php b/tests/acceptance/features/bootstrap/SettingsContext.php index 2a3aeff2f2e8c..6b60b2c317642 100644 --- a/tests/acceptance/features/bootstrap/SettingsContext.php +++ b/tests/acceptance/features/bootstrap/SettingsContext.php @@ -71,16 +71,16 @@ public static function restrictUsernameAutocompletionToGroupsCheckbox() { // forThe()->checkbox("Restrict username...") can not be used here; that // would return the checkbox itself, but the element that the user // interacts with is the label. - return Locator::forThe()->xpath("//label[normalize-space() = 'Restrict username autocompletion to users within the same groups']")-> - describedAs("Restrict username autocompletion to groups checkbox in Sharing section in Administration Sharing Settings"); + return Locator::forThe()->xpath("//label[normalize-space() = 'Allow username autocompletion to users within the same groups']")-> + describedAs("Allow username autocompletion to users within the same groups checkbox in Sharing section in Administration Sharing Settings"); } /** * @return Locator */ public static function restrictUsernameAutocompletionToGroupsCheckboxInput() { - return Locator::forThe()->checkbox("Restrict username autocompletion to users within the same groups")-> - describedAs("Restrict username autocompletion to groups checkbox input in Sharing section in Administration Sharing Settings"); + return Locator::forThe()->checkbox("Allow username autocompletion to users within the same groups")-> + describedAs("Allow username autocompletion to users within the same groups checkbox input in Sharing section in Administration Sharing Settings"); } /** From 49f7d08b38da58e028bc074b205009b78ba10c09 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 19:10:24 +0100 Subject: [PATCH 13/17] Also execute the new tests on drone Signed-off-by: Joas Schilling --- .drone.yml | 25 +++++++++++++++++++ .../autocomplete.feature | 7 ++++++ 2 files changed, 32 insertions(+) diff --git a/.drone.yml b/.drone.yml index 26124e8753ef1..dfea98a59b0dc 100644 --- a/.drone.yml +++ b/.drone.yml @@ -754,6 +754,31 @@ trigger: - pull_request - push +--- +kind: pipeline +name: integration-collaboration_features + +steps: +- name: submodules + image: docker:git + commands: + - git submodule update --init +- name: integration-collaboration_features + image: nextcloudci/integration-php7.3:integration-php7.3-2 + commands: + - bash tests/drone-run-integration-tests.sh || exit 0 + - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int + - cd build/integration + - ./run.sh collaboration_features/ + +trigger: + branch: + - master + - stable* + event: + - pull_request + - push + --- kind: pipeline name: integration-federation_features diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature index 5e294709d7fa0..e20993e420e36 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -12,6 +12,13 @@ Feature: autocomplete Given As an "admin" When get autocomplete for "auto" | id | source | + | auto | users | + | autocomplete | users | + | autocomplete2 | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "auto" + | id | source | + | auto | users | | autocomplete | users | | autocomplete2 | users | From 490bfa7330b1915234607f5903a84b8ccfa78a3e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 19:37:10 +0100 Subject: [PATCH 14/17] Also clear the knownUser when changing via the settings endpoint Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 24 ++++++++++++++++++- .../tests/Controller/UsersControllerTest.php | 6 +++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index cd34dd7266f58..81de105eabbd3 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -42,6 +42,7 @@ use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\ForbiddenException; use OC\Group\Manager as GroupManager; +use OC\KnownUser\KnownUserService; use OC\L10N\Factory; use OC\Security\IdentityProof\Manager; use OC\User\Manager as UserManager; @@ -96,6 +97,8 @@ class UsersController extends Controller { private $jobList; /** @var IManager */ private $encryptionManager; + /** @var KnownUserService */ + private $knownUserService; /** @var IEventDispatcher */ private $dispatcher; @@ -116,6 +119,7 @@ public function __construct( Manager $keyManager, IJobList $jobList, IManager $encryptionManager, + KnownUserService $knownUserService, IEventDispatcher $dispatcher ) { parent::__construct($appName, $request); @@ -132,6 +136,7 @@ public function __construct( $this->keyManager = $keyManager; $this->jobList = $jobList; $this->encryptionManager = $encryptionManager; + $this->knownUserService = $knownUserService; $this->dispatcher = $dispatcher; } @@ -363,6 +368,19 @@ public function setUserSettings(?string $avatarScope = null, ?string $twitter = null, ?string $twitterScope = null ) { + $user = $this->userSession->getUser(); + if (!$user instanceof IUser) { + return new DataResponse( + [ + 'status' => 'error', + 'data' => [ + 'message' => $this->l10n->t('Invalid user') + ] + ], + Http::STATUS_UNAUTHORIZED + ); + } + $email = strtolower($email); if (!empty($email) && !$this->mailer->validateMailAddress($email)) { return new DataResponse( @@ -375,8 +393,9 @@ public function setUserSettings(?string $avatarScope = null, Http::STATUS_UNPROCESSABLE_ENTITY ); } - $user = $this->userSession->getUser(); + $data = $this->accountManager->getUser($user); + $beforeData = $data; $data[IAccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; @@ -393,6 +412,9 @@ public function setUserSettings(?string $avatarScope = null, } try { $data = $this->saveUserSettings($user, $data); + if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { + $this->knownUserService->deleteKnownUser($user->getUID()); + } return new DataResponse( [ 'status' => 'success', diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 1a9af2ea8c9b5..b14e8d00d60bd 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -32,6 +32,7 @@ use OC\Accounts\AccountManager; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\Group\Manager; +use OC\KnownUser\KnownUserService; use OCA\Settings\Controller\UsersController; use OCP\Accounts\IAccountManager; use OCP\App\IAppManager; @@ -91,6 +92,8 @@ class UsersControllerTest extends \Test\TestCase { private $securityManager; /** @var IManager | \PHPUnit\Framework\MockObject\MockObject */ private $encryptionManager; + /** @var KnownUserService|\PHPUnit\Framework\MockObject\MockObject */ + private $knownUserService; /** @var IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject */ private $encryptionModule; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ @@ -111,6 +114,7 @@ protected function setUp(): void { $this->securityManager = $this->getMockBuilder(\OC\Security\IdentityProof\Manager::class)->disableOriginalConstructor()->getMock(); $this->jobList = $this->createMock(IJobList::class); $this->encryptionManager = $this->createMock(IManager::class); + $this->knownUserService = $this->createMock(KnownUserService::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->l->method('t') @@ -147,6 +151,7 @@ protected function getController($isAdmin = false, $mockedMethods = []) { $this->securityManager, $this->jobList, $this->encryptionManager, + $this->knownUserService, $this->dispatcher ); } else { @@ -168,6 +173,7 @@ protected function getController($isAdmin = false, $mockedMethods = []) { $this->securityManager, $this->jobList, $this->encryptionManager, + $this->knownUserService, $this->dispatcher ] )->setMethods($mockedMethods)->getMock(); From 99c1eda9c5bac39e16ef8d6b2ba4845ce6722836 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 20:05:39 +0100 Subject: [PATCH 15/17] Bump version to run migration Signed-off-by: Joas Schilling --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index d9b88f00bca09..47b23ae2f7ae9 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [21, 0, 0, 18]; +$OC_Version = [21, 0, 0, 19]; // The human readable string $OC_VersionString = '21.0.0'; From 5fa52d492af28010836ebed04027dd038c52ef39 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 20:30:29 +0100 Subject: [PATCH 16/17] Rename some parameters and methods to make the API more clear Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 2 +- .../lib/Listener/UserDeletedListener.php | 2 +- .../lib/Controller/UsersController.php | 2 +- lib/private/KnownUser/KnownUserMapper.php | 4 +- lib/private/KnownUser/KnownUserService.php | 39 +++++++++++++++---- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 198d6657468aa..f1d03a53fc551 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -679,7 +679,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon $this->accountManager->updateUser($targetUser, $userAccount, true); if ($key === IAccountManager::PROPERTY_PHONE) { - $this->knownUserService->deleteKnownUser($targetUser->getUID()); + $this->knownUserService->deleteByContactUserId($targetUser->getUID()); } } catch (\InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); diff --git a/apps/provisioning_api/lib/Listener/UserDeletedListener.php b/apps/provisioning_api/lib/Listener/UserDeletedListener.php index f4fdb973080b3..1e021177bb403 100644 --- a/apps/provisioning_api/lib/Listener/UserDeletedListener.php +++ b/apps/provisioning_api/lib/Listener/UserDeletedListener.php @@ -49,6 +49,6 @@ public function handle(Event $event): void { $this->service->deleteKnownTo($user->getUID()); // Delete all entries that other users know this user - $this->service->deleteKnownUser($user->getUID()); + $this->service->deleteByContactUserId($user->getUID()); } } diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 81de105eabbd3..02a0cda139e57 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -413,7 +413,7 @@ public function setUserSettings(?string $avatarScope = null, try { $data = $this->saveUserSettings($user, $data); if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { - $this->knownUserService->deleteKnownUser($user->getUID()); + $this->knownUserService->deleteByContactUserId($user->getUID()); } return new DataResponse( [ diff --git a/lib/private/KnownUser/KnownUserMapper.php b/lib/private/KnownUser/KnownUserMapper.php index 1144e2fd21200..e77e47527025e 100644 --- a/lib/private/KnownUser/KnownUserMapper.php +++ b/lib/private/KnownUser/KnownUserMapper.php @@ -63,10 +63,12 @@ public function deleteKnownUser(string $knownUser): int { } /** + * Returns all "known users" for the given "known to" user + * * @param string $knownTo * @return KnownUser[] */ - public function getKnownTo(string $knownTo): array { + public function getKnownUsers(string $knownTo): array { $query = $this->db->getQueryBuilder(); $query->select('*') ->from($this->getTableName()) diff --git a/lib/private/KnownUser/KnownUserService.php b/lib/private/KnownUser/KnownUserService.php index 3a97b967c3a98..96af21c836f66 100644 --- a/lib/private/KnownUser/KnownUserService.php +++ b/lib/private/KnownUser/KnownUserService.php @@ -33,30 +33,55 @@ public function __construct(KnownUserMapper $mapper) { $this->mapper = $mapper; } + /** + * Delete all matches where the given user is the owner of the phonebook + * + * @param string $knownTo + * @return int Number of deleted matches + */ public function deleteKnownTo(string $knownTo): int { return $this->mapper->deleteKnownTo($knownTo); } - public function deleteKnownUser(string $knownUser): int { - return $this->mapper->deleteKnownUser($knownUser); + /** + * Delete all matches where the given user is the one in the phonebook + * + * @param string $contactUserId + * @return int Number of deleted matches + */ + public function deleteByContactUserId(string $contactUserId): int { + return $this->mapper->deleteKnownUser($contactUserId); } - public function storeIsKnownToUser(string $knownTo, string $knownUser): void { + /** + * Store a match because $knownTo has $contactUserId in his phonebook + * + * @param string $knownTo User id of the owner of the phonebook + * @param string $contactUserId User id of the contact in the phonebook + */ + public function storeIsKnownToUser(string $knownTo, string $contactUserId): void { $entity = new KnownUser(); $entity->setKnownTo($knownTo); - $entity->setKnownUser($knownUser); + $entity->setKnownUser($contactUserId); $this->mapper->insert($entity); } - public function isKnownToUser(string $knownTo, string $user): bool { + /** + * Check if $contactUserId is in the phonebook of $knownTo + * + * @param string $knownTo User id of the owner of the phonebook + * @param string $contactUserId User id of the contact in the phonebook + * @return bool + */ + public function isKnownToUser(string $knownTo, string $contactUserId): bool { if (!isset($this->knownUsers[$knownTo])) { - $entities = $this->mapper->getKnownTo($knownTo); + $entities = $this->mapper->getKnownUsers($knownTo); $this->knownUsers[$knownTo] = []; foreach ($entities as $entity) { $this->knownUsers[$knownTo][$entity->getKnownUser()] = true; } } - return isset($this->knownUsers[$knownTo][$user]); + return isset($this->knownUsers[$knownTo][$contactUserId]); } } From 62ef45028ea3990993a3513ca37ae80c7ff8e245 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Mar 2021 20:31:12 +0100 Subject: [PATCH 17/17] Clean up the logic of the contacts store a bit Signed-off-by: Joas Schilling --- lib/private/Contacts/ContactsMenu/ContactsStore.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index e0e0bf832b330..69f26c7969fb8 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -165,21 +165,21 @@ private function filterContacts(IUser $self, return false; } - $filterUser = true; + $filterOutUser = true; $mailAddresses = $entry->getEMailAddresses(); foreach ($mailAddresses as $mailAddress) { if ($mailAddress === $filter) { - $filterUser = false; + $filterOutUser = false; break; } } if ($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) { - $filterUser = false; + $filterOutUser = false; } - if ($filterUser) { + if ($filterOutUser) { return false; } } elseif ($restrictEnumerationPhone || $restrictEnumerationGroup) { @@ -208,7 +208,7 @@ private function filterContacts(IUser $self, if ($ownGroupsOnly && !$checkedCommonGroupAlready) { $user = $this->userManager->get($entry->getProperty('UID')); - if ($user === null) { + if (!$user instanceof IUser) { return false; }