Skip to content

Commit

Permalink
code and wiring adjustments
Browse files Browse the repository at this point in the history
  • Loading branch information
jvillafanez committed Apr 29, 2020
1 parent ea8965e commit 3213f55
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 50 deletions.
7 changes: 1 addition & 6 deletions core/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,10 @@ public function __construct(array $urlParams= []) {
);
});
$container->registerService('UserSyncController', static function (SimpleContainer $c) {
$syncService = new SyncService(
$c->query(IConfig::class),
$c->query(ILogger::class),
$c->query(AccountMapper::class)
);
return new UserSyncController(
$c->query('AppName'),
$c->query('Request'),
$syncService,
$c->query(SyncService::class),
$c->query('UserManager')
);
});
Expand Down
20 changes: 6 additions & 14 deletions core/Command/User/SyncBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
use OC\User\Sync\AllUsersIterator;
use OC\User\Sync\SeenUsersIterator;
use OC\User\SyncService;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;
use OCP\UserInterface;
Expand All @@ -45,12 +43,10 @@ class SyncBackend extends Command {

/** @var AccountMapper */
protected $accountMapper;
/** @var IConfig */
private $config;
/** @var IUserManager */
private $userManager;
/** @var ILogger */
private $logger;
/** @var SyncService */
private $syncService;

/**
* @param AccountMapper $accountMapper
Expand All @@ -59,14 +55,12 @@ class SyncBackend extends Command {
* @param ILogger $logger
*/
public function __construct(AccountMapper $accountMapper,
IConfig $config,
IUserManager $userManager,
ILogger $logger) {
SyncService $syncService) {
parent::__construct();
$this->accountMapper = $accountMapper;
$this->config = $config;
$this->userManager = $userManager;
$this->logger = $logger;
$this->syncService = $syncService;
}

protected function configure() {
Expand Down Expand Up @@ -164,14 +158,12 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$missingAccountsAction = $helper->ask($input, $output, $question);
}

$syncService = new SyncService($this->config, $this->logger, $this->accountMapper);

$uid = $input->getOption('uid');

if ($uid) {
$this->syncSingleUser($input, $output, $syncService, $backend, $uid, $missingAccountsAction);
$this->syncSingleUser($input, $output, $this->syncService, $backend, $uid, $missingAccountsAction);
} else {
$this->syncMultipleUsers($input, $output, $syncService, $backend, $missingAccountsAction);
$this->syncMultipleUsers($input, $output, $this->syncService, $backend, $missingAccountsAction);
}
return 0;
}
Expand Down
6 changes: 1 addition & 5 deletions core/Migrations/Version20170221114437.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ class Version20170221114437 implements ISimpleMigration {
*/
public function run(IOutput $out) {
$backend = new Database();
$config = \OC::$server->getConfig();
$logger = \OC::$server->getLogger();
$connection = \OC::$server->getDatabaseConnection();
$accountMapper = new AccountMapper($config, $connection, new AccountTermMapper($connection));
$syncService = new SyncService($config, $logger, $accountMapper);
$syncService = \OC::$server->query(SyncService::class);

// insert/update known users
$out->info("Insert new users ...");
Expand Down
2 changes: 1 addition & 1 deletion core/register_command.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
\OC::$server->getMailer(), \OC::$server->getTimeFactory(), \OC::$server->getLogger(), \OC::$server->getUserSession())));
$application->add(new OC\Core\Command\User\Setting(\OC::$server->getUserManager(), \OC::$server->getConfig(), \OC::$server->getDatabaseConnection()));
$application->add(new OC\Core\Command\User\Modify(\OC::$server->getUserManager(), \OC::$server->getMailer()));
$application->add(new OC\Core\Command\User\SyncBackend(\OC::$server->getAccountMapper(), \OC::$server->getConfig(), \OC::$server->getUserManager(), \OC::$server->getLogger()));
$application->add(new OC\Core\Command\User\SyncBackend(\OC::$server->getAccountMapper(), \OC::$server->getUserManager(), \OC::$server->query(\OC\User\SyncService::class)));

$application->add(new OC\Core\Command\Group\Add(\OC::$server->getGroupManager()));
$application->add(new OC\Core\Command\Group\Delete(\OC::$server->getGroupManager()));
Expand Down
14 changes: 14 additions & 0 deletions lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -495,4 +495,18 @@ public function registerCapability($serviceName) {
return $this->query($serviceName);
});
}

public function query($name) {
$name = $this->sanitizeName($name);
if ($this->isServiceRegistered($name)) {
return parent::query($name);
} else {
// check server container first
if ($this->getServer()->isServiceRegistered($name)) {
return $this->getServer()->query($name);
} else {
return parent::query($name);
}
}
}
}
3 changes: 3 additions & 0 deletions lib/private/AppFramework/Utility/SimpleContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ public function registerAlias($alias, $target) {
}, false);
}

public function isServiceRegistered($name) {
return $this->offsetExists($name);
}
/*
* @param string $name
* @return string
Expand Down
18 changes: 12 additions & 6 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
use OC\User\AccountTermMapper;
use OC\User\Session;
use OC\User\SyncService;
use OC\User\UidValidator\ValidatorFactory;
use OCP\App\IServiceLoader;
use OCP\AppFramework\QueryException;
use OCP\AppFramework\Utility\ITimeFactory;
Expand Down Expand Up @@ -249,16 +250,21 @@ public function __construct($webRoot, \OC\Config $config) {
$this->registerService('AccountMapper', function (Server $c) {
return new AccountMapper($c->getConfig(), $c->getDatabaseConnection(), new AccountTermMapper($c->getDatabaseConnection()));
});
$this->registerService(SyncService::class, function (Server $c) {
$validatorFactory = $c->query(ValidatorFactory::class);
return new SyncService(
$c->getConfig(),
$c->getLogger(),
$c->getAccountMapper(),
$validatorFactory->getValidatorChainFromConfig()
);
});
$this->registerService('UserManager', function (Server $c) {
return new \OC\User\Manager(
$c->getConfig(),
$c->getLogger(),
$c->getAccountMapper(),
new SyncService(
$c->getConfig(),
$c->getLogger(),
$c->getAccountMapper()
),
$c->query(SyncService::class),
new UserSearch(
$c->getConfig()
)
Expand Down Expand Up @@ -324,7 +330,7 @@ public function __construct($webRoot, \OC\Config $config) {
$defaultTokenProvider = null;
}

$userSyncService = new SyncService($c->getConfig(), $c->getLogger(), $c->getAccountMapper());
$userSyncService = $c->query(SyncService::class);

$userSession = new Session($manager, $session, $timeFactory,
$defaultTokenProvider, $c->getConfig(), $c->getLogger(), $this, $userSyncService, $c->getEventDispatcher());
Expand Down
11 changes: 0 additions & 11 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -356,17 +356,6 @@ public function createUser($uid, $password) {
throw new \Exception($l->t('The username can not be longer than 64 characters'));
}

$invalidUids = [
'avatars',
'meta',
'files_external',
'files_encryption'
];

if (\in_array(\strtolower($uid), $invalidUids)) {
throw new \Exception($l->t("The special username $uid is not allowed"));
}

// No empty password
if (\trim($password) == '') {
throw new \Exception($l->t('A valid password must be provided'));
Expand Down
19 changes: 18 additions & 1 deletion lib/private/User/SyncService.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
namespace OC\User;

use OC\User\UidValidator\IValidator;
use OC\User\UidValidator\ValidatorException;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IConfig;
use OCP\ILogger;
Expand Down Expand Up @@ -51,6 +53,8 @@ class SyncService {
private $logger;
/** @var AccountMapper */
private $mapper;
/** @var IValidator */
private $uidValidator;

/**
* SyncService constructor.
Expand All @@ -61,10 +65,12 @@ class SyncService {
*/
public function __construct(IConfig $config,
ILogger $logger,
AccountMapper $mapper) {
AccountMapper $mapper,
IValidator $uidValidator) {
$this->config = $config;
$this->logger = $logger;
$this->mapper = $mapper;
$this->uidValidator = $uidValidator;
}

/**
Expand Down Expand Up @@ -382,6 +388,17 @@ public function createOrSyncAccount($uid, UserInterface $backend) {

// The account exists, sync
$account = $this->syncAccount($account, $backend);

// validate the uid
try {
if (!$this->uidValidator->validateUID($account->getUserId())) {
throw new \Exception("Cannot create or update account {$account->getUserId()}: invalid uid");
}
} catch (ValidatorException $e) {
$this->logger->logException($e, ['app' => self::class]);
// assume the uid is valid
}

if ($account->getId() === null) {
// New account, insert
$this->mapper->insert($account);
Expand Down
4 changes: 3 additions & 1 deletion lib/private/User/UidValidator/HardcodedValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OC\User\UidValidator;

use OC\User\UidValidator\IValidator;

/**
* This class contains a fixed set of names that cannot be changed.
*/
Expand All @@ -46,5 +47,6 @@ public function validateUID(string $uid) {
* The lists is hardcoded, so do nothing
* @inheritDoc
*/
public function reload() {}
public function reload() {
}
}
4 changes: 2 additions & 2 deletions lib/private/User/UidValidator/SystemConfigValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ public function validateUID(string $uid) {

$nameList = $this->config->getSystemValue('accounts.validators.systemconfig.rejectlist', []);
// validate data
if (!is_array($nameList)){
if (!\is_array($nameList)) {
throw new ValidatorException('Expected a list of names under accounts.validators.systemconfig.rejectlist', 1);
}

// create a set to speed up the validation process
foreach ($nameList as $name) {
if (!is_string($name)) {
if (!\is_string($name)) {
$this->nameSet = null; // reset the set, just in case.
throw new ValidatorException('The list of names contains more things than strings', 2);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/private/User/UidValidator/ValidatorChain.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ public function validateUID(string $uid) {
if (!$validator->validateUID($uid)) {
// include a debug log to make easier to know which validator rejected the uid
$this->logger->debug("{validator} rejected {uid}", [
'validator' => get_class($validator),
'validator' => \get_class($validator),
'uid' => $uid,
]);
return false;
}
} catch (ValidatorException $e) {
// remove the validator from the chain (but keep it stored)
$this->logger->error("{validator} crashed while validating {uid} with message: {message}", [
'validator' => get_class($validator),
'validator' => \get_class($validator),
'uid' => $uid,
'message' => $e->getMessage(),
]);
Expand Down
2 changes: 1 addition & 1 deletion lib/private/User/UidValidator/ValidatorFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function getValidatorChainFromConfig() {
$validatorChain->addNewValidator(new SystemConfigValidator($this->config));
break;
default:
$this->logger->warn("Ignoring UidValidator with name '$name'");
$this->logger->warning("Ignoring UidValidator with name '$name'");
break;
}
}
Expand Down
10 changes: 10 additions & 0 deletions lib/public/IContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,14 @@ public function registerService($name, Closure $closure, $shared = true);
* @since 8.2.0
*/
public function registerAlias($alias, $target);

/**
* Check if the service has been registered. The service might have been created
* or not (services are created on demand).
* This method shouldn't create the service.
* @param string $name the name of the service to check
* @return bool
* @since 10.5.0
*/
public function isServiceRegistered($name);
}

0 comments on commit 3213f55

Please sign in to comment.