Merge pull request #30638 from owncloud/stable10-sync-after-login

[stable10] Sync code deduplication and core controlled sync

apps/dav/tests/unit/Connector/Sabre/RequestTest/PartFileInRootUploadTest.php

@@ -31,24 +31,21 @@
  * @package OCA\DAV\Tests\unit\Connector\Sabre\RequestTest
  */
 class PartFileInRootUploadTest extends UploadTest {
+
+	protected $original;
+
 	protected function setUp() {
 		$config = \OC::$server->getConfig();
-		$mockConfig = $this->createMock('\OCP\IConfig');
-		$mockConfig->expects($this->any())
-			->method('getSystemValue')
-			->will($this->returnCallback(function ($key, $default) use ($config) {
-				if ($key === 'part_file_in_storage') {
-					return false;
-				} else {
-					return $config->getSystemValue($key, $default);
-				}
-			}));
-		$this->overwriteService('AllConfig', $mockConfig);
+		$this->original = $config->getSystemValue('part_file_in_storage', null);
+		$config->setSystemValue('part_file_in_storage', false);
 		parent::setUp();
 	}
 
 	protected function tearDown() {
-		$this->restoreService('AllConfig');
+		if($this->original !== null) {
+			$config = \OC::$server->getConfig();
+			$this->original = $config->setSystemValue('part_file_in_storage', $this->original);
+		}
 		return parent::tearDown();
 	}
 }

apps/testing/lib/Application.php

@@ -22,6 +22,7 @@
 namespace OCA\Testing;
 
 use OCP\AppFramework\App;
+use OCP\App\ManagerEvent;
 
 class Application extends App {
 	public function __construct (array $urlParams = array()) {
@@ -36,6 +37,37 @@ public function __construct (array $urlParams = array()) {
 			// replace all user backends with this one
 			$userManager->clearBackends();
 			$userManager->registerBackend($c->query(AlternativeHomeUserBackend::class));
+
+			$userManager->listen('\OC\User', 'postCreateUser', function ($user, $password) use ($c) {
+				$q = $c->getServer()->getDatabaseConnection()->getQueryBuilder();
+				$q->update('*PREFIX*accounts')
+					->set('backend', $q->expr()->literal(AlternativeHomeUserBackend::class))
+					->where($q->expr()->eq('user_id', $q->createNamedParameter($user->getUID())))
+					->execute();
+			});
+
+			// first call must adjust all existing backends
+			if ($config->getAppValue($appName, 'updated_all', 'no') === 'no') {
+				$q = $c->getServer()->getDatabaseConnection()->getQueryBuilder();
+				$q->update('*PREFIX*accounts')
+					->set('backend', $q->expr()->literal(AlternativeHomeUserBackend::class))
+					->where($q->expr()->eq('backend', $q->createNamedParameter(\OC\User\Database::class)))
+					->execute();
+
+				// set this value to only do it once, the value is cleared when disabling the app
+				$config->setAppValue($appName, 'updated_all', 'yes');
+			};
 		}
+
+		$eventDispatcher = $c->getServer()->getEventDispatcher();
+		$eventDispatcher->addListener(
+			ManagerEvent::EVENT_APP_DISABLE,
+			function($event) use ($appName, $config) {
+				// clear the "already updated" flag when disabling this app
+				if ($event->getAppID() === $appName) {
+					$config->deleteAppValue($appName, 'updated_all');
+				}
+			}
+		);
 	}
 }

core/Command/User/SyncBackend.php

@@ -130,10 +130,10 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 			$missingAccountsAction = $helper->ask($input, $output, $question);
 		}
 
-		$syncService = new SyncService($this->accountMapper, $backend, $this->config, $this->logger);
+		$syncService = new SyncService($this->config, $this->logger, $this->accountMapper);
 
 		// analyse unknown users
-		$this->handleUnknownUsers($input, $output, $syncService, $missingAccountsAction, $validActions);
+		$this->handleUnknownUsers($input, $output, $backend, $syncService, $missingAccountsAction, $validActions);
 
 		// insert/update known users
 		$output->writeln("Insert new and update existing users ...");
@@ -143,7 +143,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 			$max = $backend->countUsers();
 		}
 		$p->start($max);
-		$syncService->run(function () use ($p) {
+		$syncService->run($backend, function () use ($p) {
 			$p->advance();
 		});
 		$p->finish();
@@ -189,14 +189,15 @@ private function doActionForAccountUids(array $uids, callable $callbackExists, c
 	/**
 	 * @param InputInterface $input
 	 * @param OutputInterface $output
-	 * @param $syncService
+	 * @param UserInterface $backend
+	 * @param SyncService $syncService
 	 * @param $missingAccountsAction
 	 * @param $validActions
 	 */
-	private function handleUnknownUsers(InputInterface $input, OutputInterface $output, $syncService, $missingAccountsAction, $validActions) {
+	private function handleUnknownUsers(InputInterface $input, OutputInterface $output, UserInterface $backend, SyncService $syncService, $missingAccountsAction, $validActions) {
 		$output->writeln("Analyse unknown users ...");
 		$p = new ProgressBar($output);
-		$toBeDeleted = $syncService->getNoLongerExistingUsers(function () use ($p) {
+		$toBeDeleted = $syncService->getNoLongerExistingUsers($backend, function () use ($p) {
 			$p->advance();
 		});
 		$p->finish();

core/Migrations/Version20170221114437.php

@@ -19,12 +19,12 @@ public function run(IOutput $out) {
 		$logger = \OC::$server->getLogger();
 		$connection = \OC::$server->getDatabaseConnection();
 		$accountMapper = new AccountMapper($config, $connection, new AccountTermMapper($connection));
-		$syncService = new SyncService($accountMapper, $backend, $config, $logger);
+		$syncService = new SyncService($config, $logger, $accountMapper);
 
 		// insert/update known users
 		$out->info("Insert new users ...");
 		$out->startProgress($backend->countUsers());
-		$syncService->run(function () use ($out) {
+		$syncService->run($backend, function () use ($out) {
 			$out->advance();
 		});
 		$out->finishProgress();

lib/private/Server.php

@@ -85,6 +85,7 @@
 use OC\Theme\ThemeService;
 use OC\User\AccountMapper;
 use OC\User\AccountTermMapper;
+use OC\User\SyncService;
 use OCP\App\IServiceLoader;
 use OCP\AppFramework\QueryException;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -239,9 +240,16 @@ public function __construct($webRoot, \OC\Config $config) {
 			return new AccountMapper($c->getConfig(), $c->getDatabaseConnection(), new AccountTermMapper($c->getDatabaseConnection()));
 		});
 		$this->registerService('UserManager', function (Server $c) {
-			$config = $c->getConfig();
-			$logger = $c->getLogger();
-			return new \OC\User\Manager($config, $logger, $c->getAccountMapper());
+			return new \OC\User\Manager(
+				$c->getConfig(),
+				$c->getLogger(),
+				$c->getAccountMapper(),
+				new SyncService(
+					$c->getConfig(),
+					$c->getLogger(),
+					$c->getAccountMapper()
+				)
+			);
 		});
 		$this->registerService('GroupManager', function (Server $c) {
 			$groupManager = new \OC\Group\Manager($this->getUserManager());
@@ -296,8 +304,10 @@ public function __construct($webRoot, \OC\Config $config) {
 				$defaultTokenProvider = null;
 			}
 
+			$userSyncService = new SyncService($c->getConfig(), $c->getLogger(), $c->getAccountMapper());
+
 			$userSession = new \OC\User\Session($manager, $session, $timeFactory,
-				$defaultTokenProvider, $c->getConfig(), $this);
+				$defaultTokenProvider, $c->getConfig(), $this, $userSyncService);
 			$userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) {
 				\OC_Hook::emit('OC_User', 'pre_createUser', ['run' => true, 'uid' => $uid, 'password' => $password]);
 			});

lib/private/User/Account.php

@@ -28,7 +28,7 @@
 /**
  * Class Account
  *
- * @method int getUserId()
+ * @method string getUserId()
  * @method string getDisplayName()
  * @method void setDisplayName(string $displayName)
  * @method string getEmail()
@@ -71,6 +71,9 @@ public function __construct() {
 		$this->addType('lastLogin', 'integer');
 	}
 
+	/**
+	 * @param string $uid
+	 */
 	public function setUserId($uid) {
 		parent::setter('lowerUserId', [strtolower($uid)]);
 		parent::setter('userId', [$uid]);

lib/private/User/Database.php

@@ -52,12 +52,15 @@
 
 use OC\Cache\CappedMemoryCache;
 use OCP\IUserBackend;
+use OCP\User\IProvidesDisplayNameBackend;
+use OCP\User\IProvidesEMailBackend;
+use OCP\User\IProvidesHomeBackend;
 use OCP\Util;
 
 /**
  * Class for user management in a SQL Database (e.g. MySQL, SQLite)
  */
-class Database extends Backend implements IUserBackend {
+class Database extends Backend implements IUserBackend, IProvidesHomeBackend, IProvidesDisplayNameBackend {
 	/** @var CappedMemoryCache */
 	private $cache;
 
@@ -290,14 +293,10 @@ public function userExists($uid) {
 	/**
 	 * get the user's home directory
 	 * @param string $uid the username
-	 * @return string|false
+	 * @return string
 	 */
 	public function getHome($uid) {
-		if ($this->userExists($uid)) {
-			return \OC::$server->getConfig()->getSystemValue("datadirectory", \OC::$SERVERROOT . "/data") . '/' . $uid;
-		}
-
-		return false;
+		return \OC::$server->getConfig()->getSystemValue("datadirectory", \OC::$SERVERROOT . "/data") . '/' . $uid;
 	}
 
 	/**

lib/private/User/Manager.php

@@ -78,16 +78,21 @@ class Manager extends PublicEmitter implements IUserManager {
 	/** @var AccountMapper */
 	private $accountMapper;
 
+	/** @var SyncService */
+	private $syncService;
+
 	/**
 	 * @param IConfig $config
 	 * @param ILogger $logger
 	 * @param AccountMapper $accountMapper
+	 * @param SyncService $syncService
 	 */
-	public function __construct(IConfig $config, ILogger $logger, AccountMapper $accountMapper) {
+	public function __construct(IConfig $config, ILogger $logger, AccountMapper $accountMapper, SyncService $syncService) {
 		$this->config = $config;
 		$this->logger = $logger;
 		$this->accountMapper = $accountMapper;
 		$this->cachedUsers = new CappedMemoryCache();
+		$this->syncService = $syncService;
 		$cachedUsers = &$this->cachedUsers;
 		$this->listen('\OC\User', 'postDelete', function ($user) use (&$cachedUsers) {
 			/** @var \OC\User\User $user */
@@ -100,12 +105,14 @@ public function __construct(IConfig $config, ILogger $logger, AccountMapper $acc
 	 *
 	 * @param AccountMapper $mapper
 	 * @param array $backends
+	 * @param SyncService $syncService
 	 * @return array
 	 */
-	public function reset(AccountMapper $mapper, $backends) {
-		$return = [$this->accountMapper, $this->backends];
+	public function reset(AccountMapper $mapper, $backends, $syncService) {
+		$return = [$this->accountMapper, $this->backends, $this->syncService];
 		$this->accountMapper = $mapper;
 		$this->backends = $backends;
+		$this->syncService = $syncService;
 		$this->cachedUsers->clear();
 
 		return $return;
@@ -220,13 +227,7 @@ public function checkPassword($loginName, $password) {
 			if ($backend->implementsActions(Backend::CHECK_PASSWORD)) {
 				$uid = $backend->checkPassword($loginName, $password);
 				if ($uid !== false) {
-					try {
-						$account = $this->accountMapper->getByUid($uid);
-					} catch(DoesNotExistException $ex) {
-						$account = $this->newAccount($uid, $backend);
-					}
-					$this->cachedUsers->remove($account->getUserId());
-					// TODO always sync account with backend here to update displayname, email, search terms, home etc. user_ldap currently updates user metadata on login, core should take care of updating accounts on a successful login
+					$account = $this->syncService->createOrSyncAccount($uid, $backend);
 					return $this->getUserObject($account);
 				}
 			}
@@ -292,7 +293,7 @@ public function searchDisplayName($pattern, $limit = null, $offset = null) {
 	 * @param string $uid
 	 * @param string $password
 	 * @throws \Exception
-	 * @return bool|\OC\User\User the created user or false
+	 * @return bool|IUser the created user or false
 	 */
 	public function createUser($uid, $password) {
 		return $this->emittingCall(function () use (&$uid, &$password) {
@@ -337,16 +338,16 @@ public function createUser($uid, $password) {
 			if (empty($this->backends)) {
 				$this->registerBackend(new Database());
 			}
+
 			foreach ($this->backends as $backend) {
 				if ($backend->implementsActions(Backend::CREATE_USER)) {
 					$backend->createUser($uid, $password);
-					$account = $this->newAccount($uid, $backend);
-					$this->cachedUsers->remove($account->getUserId());
-					$user = $this->getUserObject($account);
-					$this->emit('\OC\User', 'postCreateUser', [$user, $password]);
-					return $user;
+					$user = $this->createUserFromBackend($uid, $password, $backend);
+					return $user === null ? false : $user;
 				}
 			}
+
+
 			return false;
 		}, ['before' => ['uid' => $uid], 'after' => ['uid' => $uid, 'password' => $password]], 'user', 'create');
 	}
@@ -355,12 +356,16 @@ public function createUser($uid, $password) {
 	 * @param string $uid
 	 * @param UserInterface $backend
 	 * @return IUser | null
+	 * @deprecated core is responsible for creating accounts, see user_ldap how it is done
 	 */
 	public function createUserFromBackend($uid, $password, $backend) {
 		return $this->emittingCall(function () use (&$uid, &$password, &$backend) {
-			$this->emit('\OC\User', 'preCreateUser', [$uid, '']);
-			$account = $this->newAccount($uid, $backend);
-			$this->cachedUsers->remove($account->getUserId());
+			$this->emit('\OC\User', 'preCreateUser', [$uid, $password]);
+			try {
+				$account = $this->syncService->createOrSyncAccount($uid, $backend);
+			} catch (\InvalidArgumentException $e) {
+				return null; // because that's what this method should do
+			}
 			$user = $this->getUserObject($account);
 			$this->emit('\OC\User', 'postCreateUser', [$user, $password]);
 			return $user;
@@ -432,55 +437,6 @@ public function getByEmail($email) {
 		}, $accounts);
 	}
 
-	/**
-	 * TODO inject OC\User\SyncService to deduplicate Account creation code
-	 * @param string $uid
-	 * @param UserInterface $backend
-	 * @return Account|\OCP\AppFramework\Db\Entity
-	 */
-	private function newAccount($uid, $backend) {
-		$account = new Account();
-		$account->setUserId($uid);
-		$account->setBackend(get_class($backend));
-		$account->setState(Account::STATE_ENABLED);
-		$account->setLastLogin(0);
-		if ($backend->implementsActions(Backend::GET_DISPLAYNAME)) {
-			$account->setDisplayName($backend->getDisplayName($uid));
-		}
-		if ($backend instanceof IProvidesEMailBackend) {
-			$email = $backend->getEMailAddress($uid);
-			if ($email !== null) {
-				$account->setEmail($email);
-			}
-		}
-		if ($backend instanceof IProvidesQuotaBackend) {
-			$quota = $backend->getQuota($uid);
-			if ($quota !== null) {
-				$account->setQuota($quota);
-			}
-		}
-		if ($backend instanceof IProvidesExtendedSearchBackend) {
-			$terms = $backend->getSearchTerms($uid);
-			if (!empty($terms)) {
-				$account->setSearchTerms($terms);
-			}
-		}
-		$home = false;
-		if ($backend->implementsActions(Backend::GET_HOME)) {
-			$home = $backend->getHome($uid);
-		}
-		if (!is_string($home) || substr($home, 0, 1) !== '/') {
-			$home = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . "/$uid";
-			$this->logger->warning(
-				"User backend ".get_class($backend)." provided no home for <$uid>, using <$home>.",
-				['app' => self::class]
-			);
-		}
-		$account->setHome($home);
-		$account = $this->accountMapper->insert($account);
-		return $account;
-	}
-
 	public function getBackend($backendClass) {
 		if (isset($this->backends[$backendClass])) {
 			return $this->backends[$backendClass];