Merge pull request #5923 from nextcloud/expire-federation-jobs

Expire federation jobs

apps/federation/lib/AppInfo/Application.php

@@ -24,75 +24,29 @@
 
 namespace OCA\Federation\AppInfo;
 
-use OCA\Federation\Controller\SettingsController;
 use OCA\Federation\DAV\FedAuth;
-use OCA\Federation\DbHandler;
 use OCA\Federation\Hooks;
 use OCA\Federation\Middleware\AddServerMiddleware;
-use OCA\Federation\SyncFederationAddressBooks;
-use OCA\Federation\TrustedServers;
-use OCP\AppFramework\IAppContainer;
+use OCP\AppFramework\App;
 use OCP\SabrePluginEvent;
 use OCP\Util;
 use Sabre\DAV\Auth\Plugin;
+use Sabre\DAV\Server;
 
-class Application extends \OCP\AppFramework\App {
+class Application extends App {
 
 	/**
 	 * @param array $urlParams
 	 */
-	public function __construct($urlParams = array()) {
+	public function __construct($urlParams = []) {
 		parent::__construct('federation', $urlParams);
-		$this->registerService();
 		$this->registerMiddleware();
 	}
 
-	private function registerService() {
-		$container = $this->getContainer();
-
-		$container->registerService('addServerMiddleware', function(IAppContainer $c) {
-			return new AddServerMiddleware(
-				$c->getAppName(),
-				\OC::$server->getL10N($c->getAppName()),
-				\OC::$server->getLogger()
-			);
-		});
-
-		$container->registerService('DbHandler', function(IAppContainer $c) {
-			return new DbHandler(
-				\OC::$server->getDatabaseConnection(),
-				\OC::$server->getL10N($c->getAppName())
-			);
-		});
-
-		$container->registerService('TrustedServers', function(IAppContainer $c) {
-			$server = $c->getServer();
-			return new TrustedServers(
-				$c->query('DbHandler'),
-				$server->getHTTPClientService(),
-				$server->getLogger(),
-				$server->getJobList(),
-				$server->getSecureRandom(),
-				$server->getConfig(),
-				$server->getEventDispatcher()
-			);
-		});
-
-		$container->registerService('SettingsController', function (IAppContainer $c) {
-			$server = $c->getServer();
-			return new SettingsController(
-				$c->getAppName(),
-				$server->getRequest(),
-				$server->getL10N($c->getAppName()),
-				$c->query('TrustedServers')
-			);
-		});
-
-	}
-
 	private function registerMiddleware() {
 		$container = $this->getContainer();
-		$container->registerMiddleware('addServerMiddleware');
+		$container->registerAlias('AddServerMiddleware', AddServerMiddleware::class);
+		$container->registerMiddleWare('AddServerMiddleware');
 	}
 
 	/**
@@ -102,7 +56,7 @@ class Application extends \OCP\AppFramework\App {
 	public function registerHooks() {
 
 		$container = $this->getContainer();
-		$hooksManager = new Hooks($container->query('TrustedServers'));
+		$hooksManager = $container->query(Hooks::class);
 
 		Util::connectHook(
 				'OCP\Share',
@@ -111,28 +65,18 @@ class Application extends \OCP\AppFramework\App {
 				'addServerHook'
 		);
 
-		$dispatcher = $this->getContainer()->getServer()->getEventDispatcher();
+		$dispatcher = $container->getServer()->getEventDispatcher();
 		$dispatcher->addListener('OCA\DAV\Connector\Sabre::authInit', function($event) use($container) {
 			if ($event instanceof SabrePluginEvent) {
-				$authPlugin = $event->getServer()->getPlugin('auth');
-				if ($authPlugin instanceof Plugin) {
-					$h = new DbHandler($container->getServer()->getDatabaseConnection(),
-							$container->getServer()->getL10N('federation')
-					);
-					$authPlugin->addBackend(new FedAuth($h));
+				$server = $event->getServer();
+				if ($server instanceof Server) {
+					$authPlugin = $server->getPlugin('auth');
+					if ($authPlugin instanceof Plugin) {
+						$authPlugin->addBackend($container->query(FedAuth::class));
+					}
 				}
 			}
 		});
 	}
 
-	/**
-	 * @return SyncFederationAddressBooks
-	 */
-	public function getSyncService() {
-		$syncService = \OC::$server->query('CardDAVSyncService');
-		$dbHandler = $this->getContainer()->query('DbHandler');
-		$discoveryService = \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
-		return new SyncFederationAddressBooks($dbHandler, $syncService, $discoveryService);
-	}
-
 }

apps/federation/lib/BackgroundJob/GetSharedSecret.php

@@ -32,8 +32,10 @@ use OC\BackgroundJob\Job;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
 use OCP\ILogger;
 use OCP\IURLGenerator;
@@ -46,7 +48,7 @@ use OCP\OCS\IDiscoveryService;
  *
  * @package OCA\Federation\Backgroundjob
  */
-class GetSharedSecret extends Job{
+class GetSharedSecret extends Job {
 
 	/** @var IClient */
 	private $httpClient;
@@ -69,6 +71,9 @@ class GetSharedSecret extends Job{
 	/** @var ILogger */
 	private $logger;
 
+	/** @var ITimeFactory */
+	private $timeFactory;
+
 	/** @var bool */
 	protected $retainJob = false;
 
@@ -76,45 +81,39 @@ class GetSharedSecret extends Job{
 
 	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
 
+	/** @var  int  30 day = 2592000sec */
+	private $maxLifespan = 2592000;
+
 	/**
 	 * RequestSharedSecret constructor.
 	 *
-	 * @param IClient $httpClient
+	 * @param IClientService $httpClientService
 	 * @param IURLGenerator $urlGenerator
 	 * @param IJobList $jobList
 	 * @param TrustedServers $trustedServers
 	 * @param ILogger $logger
 	 * @param DbHandler $dbHandler
 	 * @param IDiscoveryService $ocsDiscoveryService
+	 * @param ITimeFactory $timeFactory
 	 */
 	public function __construct(
-		IClient $httpClient = null,
-		IURLGenerator $urlGenerator = null,
-		IJobList $jobList = null,
-		TrustedServers $trustedServers = null,
-		ILogger $logger = null,
-		DbHandler $dbHandler = null,
-		IDiscoveryService $ocsDiscoveryService = null
+		IClientService $httpClientService,
+		IURLGenerator $urlGenerator,
+		IJobList $jobList,
+		TrustedServers $trustedServers,
+		ILogger $logger,
+		DbHandler $dbHandler,
+		IDiscoveryService $ocsDiscoveryService,
+		ITimeFactory $timeFactory
 	) {
-		$this->logger = $logger ? $logger : \OC::$server->getLogger();
-		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
-		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
-		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
-		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
-		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
-		if ($trustedServers) {
-			$this->trustedServers = $trustedServers;
-		} else {
-			$this->trustedServers = new TrustedServers(
-				$this->dbHandler,
-				\OC::$server->getHTTPClientService(),
-				$this->logger,
-				$this->jobList,
-				\OC::$server->getSecureRandom(),
-				\OC::$server->getConfig(),
-				\OC::$server->getEventDispatcher()
-			);
-		}
+		$this->logger = $logger;
+		$this->httpClient = $httpClientService->newClient();
+		$this->jobList = $jobList;
+		$this->urlGenerator = $urlGenerator;
+		$this->dbHandler = $dbHandler;
+		$this->ocsDiscoveryService = $ocsDiscoveryService;
+		$this->trustedServers = $trustedServers;
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -130,8 +129,10 @@ class GetSharedSecret extends Job{
 			$this->parentExecute($jobList, $logger);
 		}
 
-		if (!$this->retainJob) {
-			$jobList->remove($this, $this->argument);
+		$jobList->remove($this, $this->argument);
+
+		if ($this->retainJob) {
+			$this->reAddJob($this->argument);
 		}
 	}
 
@@ -147,14 +148,24 @@ class GetSharedSecret extends Job{
 
 	protected function run($argument) {
 		$target = $argument['url'];
+		$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
+		$currentTime = $this->timeFactory->getTime();
 		$source = $this->urlGenerator->getAbsoluteURL('/');
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
+		// kill job after 30 days of trying
+		$deadline = $currentTime - $this->maxLifespan;
+		if ($created < $deadline) {
+			$this->retainJob = false;
+			$this->trustedServers->setServerStatus($target,TrustedServers::STATUS_FAILURE);
+			return;
+		}
+
 		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
 		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;
 
-		// make sure that we have a well formated url
+		// make sure that we have a well formatted url
 		$url = rtrim($target, '/') . '/' . trim($endPoint, '/') . $this->format;
 
 		$result = null;
@@ -215,4 +226,23 @@ class GetSharedSecret extends Job{
 		}
 
 	}
+
+	/**
+	 * re-add background job
+	 *
+	 * @param array $argument
+	 */
+	protected function reAddJob(array $argument) {
+		$url = $argument['url'];
+		$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
+		$token = $argument['token'];
+		$this->jobList->add(
+			GetSharedSecret::class,
+			[
+				'url' => $url,
+				'token' => $token,
+				'created' => $created
+			]
+		);
+	}
 }

apps/federation/lib/BackgroundJob/RequestSharedSecret.php

@@ -33,8 +33,10 @@ use OC\BackgroundJob\Job;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
 use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\OCS\IDiscoveryService;
@@ -69,6 +71,9 @@ class RequestSharedSecret extends Job {
 	/** @var ILogger */
 	private $logger;
 
+	/** @var ITimeFactory */
+	private $timeFactory;
+
 	/** @var bool */
 	protected $retainJob = false;
 
@@ -76,43 +81,39 @@ class RequestSharedSecret extends Job {
 
 	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';
 
+	/** @var  int  30 day = 2592000sec */
+	private $maxLifespan = 2592000;
+
 	/**
 	 * RequestSharedSecret constructor.
 	 *
-	 * @param IClient $httpClient
+	 * @param IClientService $httpClientService
 	 * @param IURLGenerator $urlGenerator
 	 * @param IJobList $jobList
 	 * @param TrustedServers $trustedServers
 	 * @param DbHandler $dbHandler
 	 * @param IDiscoveryService $ocsDiscoveryService
+	 * @param ILogger $logger
+	 * @param ITimeFactory $timeFactory
 	 */
 	public function __construct(
-		IClient $httpClient = null,
-		IURLGenerator $urlGenerator = null,
-		IJobList $jobList = null,
-		TrustedServers $trustedServers = null,
-		DbHandler $dbHandler = null,
-		IDiscoveryService $ocsDiscoveryService = null
+		IClientService $httpClientService,
+		IURLGenerator $urlGenerator,
+		IJobList $jobList,
+		TrustedServers $trustedServers,
+		DbHandler $dbHandler,
+		IDiscoveryService $ocsDiscoveryService,
+		ILogger $logger,
+		ITimeFactory $timeFactory
 	) {
-		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
-		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
-		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
-		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
-		$this->logger = \OC::$server->getLogger();
-		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
-		if ($trustedServers) {
-			$this->trustedServers = $trustedServers;
-		} else {
-			$this->trustedServers = new TrustedServers(
-				$this->dbHandler,
-				\OC::$server->getHTTPClientService(),
-				$this->logger,
-				$this->jobList,
-				\OC::$server->getSecureRandom(),
-				\OC::$server->getConfig(),
-				\OC::$server->getEventDispatcher()
-			);
-		}
+		$this->httpClient = $httpClientService->newClient();
+		$this->jobList = $jobList;
+		$this->urlGenerator = $urlGenerator;
+		$this->dbHandler = $dbHandler;
+		$this->logger = $logger;
+		$this->ocsDiscoveryService = $ocsDiscoveryService;
+		$this->trustedServers = $trustedServers;
+		$this->timeFactory = $timeFactory;
 	}
 
 
@@ -129,8 +130,10 @@ class RequestSharedSecret extends Job {
 			$this->parentExecute($jobList, $logger);
 		}
 
-		if (!$this->retainJob) {
-			$jobList->remove($this, $this->argument);
+		$jobList->remove($this, $this->argument);
+
+		if ($this->retainJob) {
+			$this->reAddJob($this->argument);
 		}
 	}
 
@@ -147,10 +150,20 @@ class RequestSharedSecret extends Job {
 	protected function run($argument) {
 
 		$target = $argument['url'];
+		$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
+		$currentTime = $this->timeFactory->getTime();
 		$source = $this->urlGenerator->getAbsoluteURL('/');
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
+		// kill job after 30 days of trying
+		$deadline = $currentTime - $this->maxLifespan;
+		if ($created < $deadline) {
+			$this->retainJob = false;
+			$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
+			return;
+		}
+
 		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
 		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;
 
@@ -198,4 +211,24 @@ class RequestSharedSecret extends Job {
 		}
 
 	}
+
+	/**
+	 * re-add background job
+	 *
+	 * @param array $argument
+	 */
+	protected function reAddJob(array $argument) {
+		$url = $argument['url'];
+		$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
+		$token = $argument['token'];
+
+		$this->jobList->add(
+			RequestSharedSecret::class,
+			[
+				'url' => $url,
+				'token' => $token,
+				'created' => $created
+			]
+		);
+	}
 }

apps/federation/lib/Command/SyncFederationAddressBooks.php

@@ -36,7 +36,7 @@ class SyncFederationAddressBooks extends Command {
 	/**
 	 * @param \OCA\Federation\SyncFederationAddressBooks $syncService
 	 */
-	function __construct(\OCA\Federation\SyncFederationAddressBooks $syncService) {
+	public function __construct(\OCA\Federation\SyncFederationAddressBooks $syncService) {
 		parent::__construct();
 
 		$this->syncService = $syncService;

apps/federation/lib/Controller/OCSAuthAPIController.php

@@ -32,6 +32,7 @@ use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCSController;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\ILogger;
 use OCP\IRequest;
@@ -61,6 +62,9 @@ class OCSAuthAPIController extends OCSController{
 	/** @var ILogger */
 	private $logger;
 
+	/** @var ITimeFactory */
+	private $timeFactory;
+
 	/**
 	 * OCSAuthAPI constructor.
 	 *
@@ -71,6 +75,7 @@ class OCSAuthAPIController extends OCSController{
 	 * @param TrustedServers $trustedServers
 	 * @param DbHandler $dbHandler
 	 * @param ILogger $logger
+	 * @param ITimeFactory $timeFactory
 	 */
 	public function __construct(
 		$appName,
@@ -79,7 +84,8 @@ class OCSAuthAPIController extends OCSController{
 		IJobList $jobList,
 		TrustedServers $trustedServers,
 		DbHandler $dbHandler,
-		ILogger $logger
+		ILogger $logger,
+		ITimeFactory $timeFactory
 	) {
 		parent::__construct($appName, $request);
 
@@ -88,6 +94,7 @@ class OCSAuthAPIController extends OCSController{
 		$this->trustedServers = $trustedServers;
 		$this->dbHandler = $dbHandler;
 		$this->logger = $logger;
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -149,20 +156,12 @@ class OCSAuthAPIController extends OCSController{
 			throw new OCSForbiddenException();
 		}
 
-		// we ask for the shared secret so we no longer have to ask the other server
-		// to request the shared secret
-		$this->jobList->remove('OCA\Federation\BackgroundJob\RequestSharedSecret',
-			[
-				'url' => $url,
-				'token' => $localToken
-			]
-		);
-
 		$this->jobList->add(
 			'OCA\Federation\BackgroundJob\GetSharedSecret',
 			[
 				'url' => $url,
 				'token' => $token,
+				'created' => $this->timeFactory->getTime()
 			]
 		);
 
@@ -210,5 +209,4 @@ class OCSAuthAPIController extends OCSController{
 		$storedToken = $this->dbHandler->getToken($url);
 		return hash_equals($storedToken, $token);
 	}
-
 }

apps/federation/lib/Controller/SettingsController.php

@@ -26,7 +26,6 @@ namespace OCA\Federation\Controller;
 use OC\HintException;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Controller;
-use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IL10N;
 use OCP\IRequest;

apps/federation/lib/DAV/FedAuth.php

@@ -63,6 +63,6 @@ class FedAuth extends AbstractBasic {
 	/**
 	 * @inheritdoc
 	 */
-	function challenge(RequestInterface $request, ResponseInterface $response) {
+	public function challenge(RequestInterface $request, ResponseInterface $response) {
 	}
 }

apps/federation/lib/DbHandler.php

@@ -88,11 +88,11 @@ class DbHandler {
 
 		if ($result) {
 			return (int)$this->connection->lastInsertId('*PREFIX*'.$this->dbTable);
-		} else {
-			$message = 'Internal failure, Could not add trusted server: ' . $url;
-			$message_t = $this->IL10N->t('Could not add server');
-			throw new HintException($message, $message_t);
 		}
+
+		$message = 'Internal failure, Could not add trusted server: ' . $url;
+		$message_t = $this->IL10N->t('Could not add server');
+		throw new HintException($message, $message_t);
 	}
 
 	/**
@@ -137,8 +137,11 @@ class DbHandler {
 	 */
 	public function getAllServer() {
 		$query = $this->connection->getQueryBuilder();
-		$query->select(['url', 'url_hash', 'id', 'status', 'shared_secret', 'sync_token'])->from($this->dbTable);
-		$result = $query->execute()->fetchAll();
+		$query->select(['url', 'url_hash', 'id', 'status', 'shared_secret', 'sync_token'])
+			->from($this->dbTable);
+		$statement = $query->execute();
+		$result = $statement->fetchAll();
+		$statement->closeCursor();
 		return $result;
 	}
 
@@ -151,10 +154,13 @@ class DbHandler {
 	public function serverExists($url) {
 		$hash = $this->hash($url);
 		$query = $this->connection->getQueryBuilder();
-		$query->select('url')->from($this->dbTable)
+		$query->select('url')
+			->from($this->dbTable)
 			->where($query->expr()->eq('url_hash', $query->createParameter('url_hash')))
 			->setParameter('url_hash', $hash);
-		$result = $query->execute()->fetchAll();
+		$statement = $query->execute();
+		$result = $statement->fetchAll();
+		$statement->closeCursor();
 
 		return !empty($result);
 	}
@@ -190,7 +196,9 @@ class DbHandler {
 			->where($query->expr()->eq('url_hash', $query->createParameter('url_hash')))
 			->setParameter('url_hash', $hash);
 
-		$result = $query->execute()->fetch();
+		$statement = $query->execute();
+		$result = $statement->fetch();
+		$statement->closeCursor();
 
 		if (!isset($result['token'])) {
 			throw new \Exception('No token found for: ' . $url);
@@ -229,7 +237,9 @@ class DbHandler {
 			->where($query->expr()->eq('url_hash', $query->createParameter('url_hash')))
 			->setParameter('url_hash', $hash);
 
-		$result = $query->execute()->fetch();
+		$statement = $query->execute();
+		$result = $statement->fetch();
+		$statement->closeCursor();
 		return $result['shared_secret'];
 	}
 
@@ -265,7 +275,9 @@ class DbHandler {
 				->where($query->expr()->eq('url_hash', $query->createParameter('url_hash')))
 				->setParameter('url_hash', $hash);
 
-		$result = $query->execute()->fetch();
+		$statement = $query->execute();
+		$result = $statement->fetch();
+		$statement->closeCursor();
 		return (int)$result['status'];
 	}
 
@@ -314,7 +326,9 @@ class DbHandler {
 		$query->select('url')->from($this->dbTable)
 				->where($query->expr()->eq('shared_secret', $query->createNamedParameter($password)));
 
-		$result = $query->execute()->fetch();
+		$statement = $query->execute();
+		$result = $statement->fetch();
+		$statement->closeCursor();
 		return !empty($result);
 	}
 

apps/federation/lib/Middleware/AddServerMiddleware.php

@@ -44,6 +44,11 @@ class AddServerMiddleware extends Middleware {
 	/** @var  ILogger */
 	protected $logger;
 
+	/**
+	 * @param string $appName
+	 * @param IL10N $l
+	 * @param ILogger $logger
+	 */
 	public function __construct($appName, IL10N $l, ILogger $logger) {
 		$this->appName = $appName;
 		$this->l = $l;

apps/federation/lib/SyncJob.php

@@ -24,20 +24,31 @@ namespace OCA\Federation;
 
 use OC\BackgroundJob\TimedJob;
 use OCA\Federation\AppInfo\Application;
+use OCP\ILogger;
 
 class SyncJob extends TimedJob {
 
-	public function __construct() {
+	/** @var SyncFederationAddressBooks */
+	protected $syncService;
+
+	/** @var ILogger */
+	protected $logger;
+
+	/**
+	 * @param SyncFederationAddressBooks $syncService
+	 * @param ILogger $logger
+	 */
+	public function __construct(SyncFederationAddressBooks $syncService, ILogger $logger) {
 		// Run once a day
 		$this->setInterval(24 * 60 * 60);
+		$this->syncService = $syncService;
+		$this->logger = $logger;
 	}
 
 	protected function run($argument) {
-		$app = new Application();
-		$ss = $app->getSyncService();
-		$ss->syncThemAll(function($url, $ex) {
+		$this->syncService->syncThemAll(function($url, $ex) {
 			if ($ex instanceof \Exception) {
-				\OC::$server->getLogger()->error("Error while syncing $url : " . $ex->getMessage(), ['app' => 'fed-sync']);
+				$this->logger->error("Error while syncing $url : " . $ex->getMessage(), ['app' => 'fed-sync']);
 			}
 		});
 	}

apps/federation/lib/TrustedServers.php

@@ -28,6 +28,7 @@ namespace OCA\Federation;
 
 use OC\HintException;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClientService;
 use OCP\IConfig;
@@ -68,6 +69,9 @@ class TrustedServers {
 	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
+	/** @var ITimeFactory */
+	private $timeFactory;
+
 	/**
 	 * @param DbHandler $dbHandler
 	 * @param IClientService $httpClientService
@@ -76,6 +80,7 @@ class TrustedServers {
 	 * @param ISecureRandom $secureRandom
 	 * @param IConfig $config
 	 * @param EventDispatcherInterface $dispatcher
+	 * @param ITimeFactory $timeFactory
 	 */
 	public function __construct(
 		DbHandler $dbHandler,
@@ -84,7 +89,8 @@ class TrustedServers {
 		IJobList $jobList,
 		ISecureRandom $secureRandom,
 		IConfig $config,
-		EventDispatcherInterface $dispatcher
+		EventDispatcherInterface $dispatcher,
+		ITimeFactory $timeFactory
 	) {
 		$this->dbHandler = $dbHandler;
 		$this->httpClientService = $httpClientService;
@@ -93,6 +99,7 @@ class TrustedServers {
 		$this->secureRandom = $secureRandom;
 		$this->config = $config;
 		$this->dispatcher = $dispatcher;
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -111,7 +118,8 @@ class TrustedServers {
 				'OCA\Federation\BackgroundJob\RequestSharedSecret',
 				[
 					'url' => $url,
-					'token' => $token
+					'token' => $token,
+					'created' => $this->timeFactory->getTime()
 				]
 			);
 		}

apps/federation/tests/BackgroundJob/GetSharedSecretTest.php

@@ -31,8 +31,10 @@ use OCA\Files_Sharing\Tests\TestCase;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
 use OCP\ILogger;
 use OCP\IURLGenerator;
@@ -47,36 +49,43 @@ use OCP\OCS\IDiscoveryService;
  */
 class GetSharedSecretTest extends TestCase {
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IClient */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClient */
 	private $httpClient;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IJobList */
+	/** @var  \PHPUnit_Framework_MockObject_MockObject|IClientService */
+	private $httpClientService;
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IJobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IURLGenerator */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IURLGenerator */
 	private $urlGenerator;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers  */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers  */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
 	private $logger;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IResponse */
 	private $response;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IDiscoveryService */
 	private $discoverService;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	/** @var GetSharedSecret */
 	private $getSharedSecret;
 
 	public function setUp() {
 		parent::setUp();
 
+		$this->httpClientService = $this->createMock(IClientService::class);
 		$this->httpClient = $this->getMockBuilder(IClient::class)->getMock();
 		$this->jobList = $this->getMockBuilder(IJobList::class)->getMock();
 		$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock();
@@ -87,17 +96,20 @@ class GetSharedSecretTest extends TestCase {
 		$this->logger = $this->getMockBuilder(ILogger::class)->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
 		$this->discoverService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->discoverService->expects($this->any())->method('discover')->willReturn([]);
+		$this->httpClientService->expects($this->any())->method('newClient')->willReturn($this->httpClient);
 
 		$this->getSharedSecret = new GetSharedSecret(
-			$this->httpClient,
+			$this->httpClientService,
 			$this->urlGenerator,
 			$this->jobList,
 			$this->trustedServers,
 			$this->logger,
 			$this->dbHandler,
-			$this->discoverService
+			$this->discoverService,
+			$this->timeFactory
 		);
 	}
 
@@ -112,16 +124,17 @@ class GetSharedSecretTest extends TestCase {
 		$getSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\GetSharedSecret')
 			->setConstructorArgs(
 				[
-					$this->httpClient,
+					$this->httpClientService,
 					$this->urlGenerator,
 					$this->jobList,
 					$this->trustedServers,
 					$this->logger,
 					$this->dbHandler,
-					$this->discoverService
+					$this->discoverService,
+					$this->timeFactory
 				]
 			)->setMethods(['parentExecute'])->getMock();
-		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url']]);
+		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
 		$this->trustedServers->expects($this->once())->method('isTrustedServer')
 			->with('url')->willReturn($isTrustedServer);
@@ -131,10 +144,23 @@ class GetSharedSecretTest extends TestCase {
 			$getSharedSecret->expects($this->never())->method('parentExecute');
 		}
 		$this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]);
+		$this->jobList->expects($this->once())->method('remove');
+
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		if ($retainBackgroundJob) {
-			$this->jobList->expects($this->never())->method('remove');
+			$this->jobList->expects($this->once())
+				->method('add')
+				->with(
+					GetSharedSecret::class,
+					[
+						'url' => 'url',
+						'token' => 'token',
+						'created' => 42,
+					]
+				);
 		} else {
-			$this->jobList->expects($this->once())->method('remove');
+			$this->jobList->expects($this->never())->method('add');
 		}
 
 		$getSharedSecret->execute($this->jobList);
@@ -155,13 +181,15 @@ class GetSharedSecretTest extends TestCase {
 	 * @param int $statusCode
 	 */
 	public function testRun($statusCode) {
-
 		$target = 'targetURL';
 		$source = 'sourceURL';
 		$token = 'token';
 
 		$argument = ['url' => $target, 'token' => $token];
 
+		$this->timeFactory->method('getTime')
+			->willReturn(42);
+
 		$this->urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('/')
 			->willReturn($source);
 		$this->httpClient->expects($this->once())->method('get')
@@ -208,7 +236,6 @@ class GetSharedSecretTest extends TestCase {
 		} else {
 			$this->assertFalse($this->invokePrivate($this->getSharedSecret, 'retainJob'));
 		}
-
 	}
 
 	public function dataTestRun() {
@@ -219,4 +246,33 @@ class GetSharedSecretTest extends TestCase {
 		];
 	}
 
+	public function testRunExpired() {
+		$target = 'targetURL';
+		$source = 'sourceURL';
+		$token = 'token';
+		$created = 42;
+
+		$argument = [
+			'url' => $target,
+			'token' => $token,
+			'created' => $created,
+		];
+
+		$this->urlGenerator->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('/')
+			->willReturn($source);
+
+		$this->timeFactory->method('getTime')
+			->willReturn($created + 2592000 + 1);
+
+		$this->trustedServers->expects($this->once())
+			->method('setServerStatus')
+			->with(
+				$target,
+				TrustedServers::STATUS_FAILURE
+			);
+
+		$this->invokePrivate($this->getSharedSecret, 'run', [$argument]);
+	}
 }

apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php

@@ -29,42 +29,55 @@ use OCA\Federation\BackgroundJob\RequestSharedSecret;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
+use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\OCS\IDiscoveryService;
 use Test\TestCase;
 
 class RequestSharedSecretTest extends TestCase {
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IClient */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClientService */
+	private $httpClientService;
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IClient */
 	private $httpClient;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IJobList */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IJobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IURLGenerator */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IURLGenerator */
 	private $urlGenerator;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IResponse */
 	private $response;
 
-	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IDiscoveryService */
 	private $discoveryService;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
+	private $logger;
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	/** @var  RequestSharedSecret */
 	private $requestSharedSecret;
 
 	public function setUp() {
 		parent::setUp();
 
+		$this->httpClientService = $this->createMock(IClientService::class);
 		$this->httpClient = $this->getMockBuilder(IClient::class)->getMock();
 		$this->jobList = $this->getMockBuilder(IJobList::class)->getMock();
 		$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock();
@@ -74,16 +87,21 @@ class RequestSharedSecretTest extends TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
 		$this->discoveryService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
+		$this->logger = $this->createMock(ILogger::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
+		$this->httpClientService->expects($this->any())->method('newClient')->willReturn($this->httpClient);
 
 		$this->requestSharedSecret = new RequestSharedSecret(
-			$this->httpClient,
+			$this->httpClientService,
 			$this->urlGenerator,
 			$this->jobList,
 			$this->trustedServers,
 			$this->dbHandler,
-			$this->discoveryService
+			$this->discoveryService,
+			$this->logger,
+			$this->timeFactory
 		);
 	}
 
@@ -98,15 +116,17 @@ class RequestSharedSecretTest extends TestCase {
 		$requestSharedSecret = $this->getMockBuilder('OCA\Federation\BackgroundJob\RequestSharedSecret')
 			->setConstructorArgs(
 				[
-					$this->httpClient,
+					$this->httpClientService,
 					$this->urlGenerator,
 					$this->jobList,
 					$this->trustedServers,
 					$this->dbHandler,
-					$this->discoveryService
+					$this->discoveryService,
+					$this->logger,
+					$this->timeFactory
 				]
 			)->setMethods(['parentExecute'])->getMock();
-		$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url']]);
+		$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
 
 		$this->trustedServers->expects($this->once())->method('isTrustedServer')
 			->with('url')->willReturn($isTrustedServer);
@@ -116,10 +136,23 @@ class RequestSharedSecretTest extends TestCase {
 			$requestSharedSecret->expects($this->never())->method('parentExecute');
 		}
 		$this->invokePrivate($requestSharedSecret, 'retainJob', [$retainBackgroundJob]);
+		$this->jobList->expects($this->once())->method('remove');
+
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		if ($retainBackgroundJob) {
-			$this->jobList->expects($this->never())->method('remove');
+			$this->jobList->expects($this->once())
+				->method('add')
+				->with(
+					RequestSharedSecret::class,
+					[
+						'url' => 'url',
+						'token' => 'token',
+						'created' => 42,
+					]
+				);
 		} else {
-			$this->jobList->expects($this->once())->method('remove');
+			$this->jobList->expects($this->never())->method('add');
 		}
 
 		$requestSharedSecret->execute($this->jobList);
@@ -147,6 +180,8 @@ class RequestSharedSecretTest extends TestCase {
 
 		$argument = ['url' => $target, 'token' => $token];
 
+		$this->timeFactory->method('getTime')->willReturn(42);
+
 		$this->urlGenerator->expects($this->once())->method('getAbsoluteURL')->with('/')
 			->willReturn($source);
 		$this->httpClient->expects($this->once())->method('post')
@@ -195,4 +230,34 @@ class RequestSharedSecretTest extends TestCase {
 			[Http::STATUS_CONFLICT],
 		];
 	}
+
+	public function testRunExpired() {
+		$target = 'targetURL';
+		$source = 'sourceURL';
+		$token = 'token';
+		$created = 42;
+
+		$argument = [
+			'url' => $target,
+			'token' => $token,
+			'created' => $created,
+		];
+
+		$this->urlGenerator->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('/')
+			->willReturn($source);
+
+		$this->timeFactory->method('getTime')
+			->willReturn($created + 2592000 + 1);
+
+		$this->trustedServers->expects($this->once())
+			->method('setServerStatus')
+			->with(
+				$target,
+				TrustedServers::STATUS_FAILURE
+			);
+
+		$this->invokePrivate($this->requestSharedSecret, 'run', [$argument]);
+	}
 }

apps/federation/tests/Controller/OCSAuthAPIControllerTest.php

@@ -29,8 +29,8 @@ use OC\BackgroundJob\JobList;
 use OCA\Federation\Controller\OCSAuthAPIController;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
-use OCP\AppFramework\Http;
 use OCP\AppFramework\OCS\OCSForbiddenException;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
@@ -38,40 +38,45 @@ use Test\TestCase;
 
 class OCSAuthAPIControllerTest extends TestCase {
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | IRequest */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|IRequest */
 	private $request;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ISecureRandom  */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ISecureRandom  */
 	private $secureRandom;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | JobList */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|JobList */
 	private $jobList;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | TrustedServers */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|TrustedServers */
 	private $trustedServers;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|DbHandler */
 	private $dbHandler;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
 	private $logger;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
+
 	/** @var  OCSAuthAPIController */
 	private $ocsAuthApi;
 
+	/** @var int simulated timestamp */
+	private $currentTime = 1234567;
+
 	public function setUp() {
 		parent::setUp();
 
-		$this->request = $this->getMockBuilder('OCP\IRequest')->getMock();
-		$this->secureRandom = $this->getMockBuilder('OCP\Security\ISecureRandom')->getMock();
-		$this->trustedServers = $this->getMockBuilder('OCA\Federation\TrustedServers')
-			->disableOriginalConstructor()->getMock();
-		$this->dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')
-			->disableOriginalConstructor()->getMock();
-		$this->jobList = $this->getMockBuilder('OC\BackgroundJob\JobList')
-			->disableOriginalConstructor()->getMock();
-		$this->logger = $this->getMockBuilder('OCP\ILogger')
-			->disableOriginalConstructor()->getMock();
+		$this->request = $this->createMock(IRequest::class);
+		$this->secureRandom = $this->createMock(ISecureRandom::class);
+		$this->trustedServers = $this->createMock(TrustedServers::class);
+		$this->dbHandler = $this->createMock(DbHandler::class);
+		$this->jobList = $this->createMock(JobList::class);
+		$this->logger = $this->createMock(ILogger::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
+
 
 		$this->ocsAuthApi = new OCSAuthAPIController(
 			'federation',
@@ -80,9 +85,13 @@ class OCSAuthAPIControllerTest extends TestCase {
 			$this->jobList,
 			$this->trustedServers,
 			$this->dbHandler,
-			$this->logger
+			$this->logger,
+			$this->timeFactory
 		);
 
+		$this->timeFactory->method('getTime')
+			->willReturn($this->currentTime);
+
 	}
 
 	/**
@@ -105,9 +114,7 @@ class OCSAuthAPIControllerTest extends TestCase {
 
 		if ($ok) {
 			$this->jobList->expects($this->once())->method('add')
-				->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token]);
-			$this->jobList->expects($this->once())->method('remove')
-				->with('OCA\Federation\BackgroundJob\RequestSharedSecret', ['url' => $url, 'token' => $localToken]);
+				->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token, 'created' => $this->currentTime]);
 		} else {
 			$this->jobList->expects($this->never())->method('add');
 			$this->jobList->expects($this->never())->method('remove');
@@ -151,7 +158,8 @@ class OCSAuthAPIControllerTest extends TestCase {
 					$this->jobList,
 					$this->trustedServers,
 					$this->dbHandler,
-					$this->logger
+					$this->logger,
+					$this->timeFactory
 				]
 			)->setMethods(['isValidToken'])->getMock();
 

apps/federation/tests/TrustedServersTest.php

@@ -29,6 +29,7 @@ namespace OCA\Federation\Tests;
 
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
@@ -71,6 +72,9 @@ class TrustedServersTest extends TestCase {
 	/** @var  \PHPUnit_Framework_MockObject_MockObject | EventDispatcherInterface */
 	private $dispatcher;
 
+	/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
+	private $timeFactory;
+
 	public function setUp() {
 		parent::setUp();
 
@@ -85,6 +89,7 @@ class TrustedServersTest extends TestCase {
 		$this->jobList = $this->getMockBuilder(IJobList::class)->getMock();
 		$this->secureRandom = $this->getMockBuilder(ISecureRandom::class)->getMock();
 		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->trustedServers = new TrustedServers(
 			$this->dbHandler,
@@ -93,7 +98,8 @@ class TrustedServersTest extends TestCase {
 			$this->jobList,
 			$this->secureRandom,
 			$this->config,
-			$this->dispatcher
+			$this->dispatcher,
+			$this->timeFactory
 		);
 
 	}
@@ -114,13 +120,16 @@ class TrustedServersTest extends TestCase {
 					$this->jobList,
 					$this->secureRandom,
 					$this->config,
-					$this->dispatcher
+					$this->dispatcher,
+					$this->timeFactory
 				]
 			)
 			->setMethods(['normalizeUrl', 'updateProtocol'])
 			->getMock();
 		$trustedServers->expects($this->once())->method('updateProtocol')
 				->with('url')->willReturn('https://url');
+		$this->timeFactory->method('getTime')
+			->willReturn(1234567);
 		$this->dbHandler->expects($this->once())->method('addServer')->with('https://url')
 			->willReturn($success);
 
@@ -130,7 +139,7 @@ class TrustedServersTest extends TestCase {
 			$this->dbHandler->expects($this->once())->method('addToken')->with('https://url', 'token');
 			$this->jobList->expects($this->once())->method('add')
 				->with('OCA\Federation\BackgroundJob\RequestSharedSecret',
-						['url' => 'https://url', 'token' => 'token']);
+						['url' => 'https://url', 'token' => 'token', 'created' => 1234567]);
 		} else {
 			$this->jobList->expects($this->never())->method('add');
 		}
@@ -272,7 +281,8 @@ class TrustedServersTest extends TestCase {
 					$this->jobList,
 					$this->secureRandom,
 					$this->config,
-					$this->dispatcher
+					$this->dispatcher,
+					$this->timeFactory
 				]
 			)
 			->setMethods(['checkOwnCloudVersion'])