Use ITimeFactory

* Inject the timefacotry so we can mock it properly in the tests.
* Extended unit tests to cover the new paths

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This commit is contained in:
Roeland Jago Douma 2017-08-01 11:23:40 +02:00
parent 3ffff08819
commit bc54e6ff7c
No known key found for this signature in database
GPG Key ID: F941078878347C0C
9 changed files with 230 additions and 102 deletions

View File

@ -32,6 +32,7 @@ use OCA\Federation\Middleware\AddServerMiddleware;
use OCA\Federation\SyncFederationAddressBooks;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\IAppContainer;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\SabrePluginEvent;
use OCP\Util;
use Sabre\DAV\Auth\Plugin;
@ -74,7 +75,8 @@ class Application extends \OCP\AppFramework\App {
$server->getJobList(),
$server->getSecureRandom(),
$server->getConfig(),
$server->getEventDispatcher()
$server->getEventDispatcher(),
$server->query(ITimeFactory::class)
);
});

View File

@ -32,6 +32,7 @@ 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;
@ -70,6 +71,9 @@ class GetSharedSecret extends Job{
/** @var ILogger */
private $logger;
/** @var ITimeFactory */
private $timeFactory;
/** @var bool */
protected $retainJob = false;
@ -90,6 +94,7 @@ class GetSharedSecret extends Job{
* @param ILogger $logger
* @param DbHandler $dbHandler
* @param IDiscoveryService $ocsDiscoveryService
* @param ITimeFactory $timeFactory
*/
public function __construct(
IClientService $httpClientService,
@ -98,7 +103,8 @@ class GetSharedSecret extends Job{
TrustedServers $trustedServers,
ILogger $logger,
DbHandler $dbHandler,
IDiscoveryService $ocsDiscoveryService
IDiscoveryService $ocsDiscoveryService,
ITimeFactory $timeFactory
) {
$this->logger = $logger;
$this->httpClient = $httpClientService->newClient();
@ -107,6 +113,7 @@ class GetSharedSecret extends Job{
$this->dbHandler = $dbHandler;
$this->ocsDiscoveryService = $ocsDiscoveryService;
$this->trustedServers = $trustedServers;
$this->timeFactory = $timeFactory;
}
/**
@ -125,7 +132,7 @@ class GetSharedSecret extends Job{
$jobList->remove($this, $this->argument);
if ($this->retainJob) {
$this->reAddJob($jobList, $this->argument);
$this->reAddJob($this->argument);
}
}
@ -141,8 +148,8 @@ class GetSharedSecret extends Job{
protected function run($argument) {
$target = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : time();
$currentTime = time();
$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
$currentTime = $this->timeFactory->getTime();
$source = $this->urlGenerator->getAbsoluteURL('/');
$source = rtrim($source, '/');
$token = $argument['token'];
@ -158,7 +165,7 @@ class GetSharedSecret extends Job{
$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;
@ -223,12 +230,11 @@ class GetSharedSecret extends Job{
/**
* re-add background job
*
* @param IJobList $jobList
* @param array $argument
*/
protected function reAddJob(IJobList $jobList, array $argument) {
protected function reAddJob(array $argument) {
$url = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : time();
$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
$token = $argument['token'];
$this->jobList->add(
GetSharedSecret::class,

View File

@ -33,6 +33,7 @@ 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;
@ -70,6 +71,9 @@ class RequestSharedSecret extends Job {
/** @var ILogger */
private $logger;
/** @var ITimeFactory */
private $timeFactory;
/** @var bool */
protected $retainJob = false;
@ -90,6 +94,7 @@ class RequestSharedSecret extends Job {
* @param DbHandler $dbHandler
* @param IDiscoveryService $ocsDiscoveryService
* @param ILogger $logger
* @param ITimeFactory $timeFactory
*/
public function __construct(
IClientService $httpClientService,
@ -98,7 +103,8 @@ class RequestSharedSecret extends Job {
TrustedServers $trustedServers,
DbHandler $dbHandler,
IDiscoveryService $ocsDiscoveryService,
ILogger $logger
ILogger $logger,
ITimeFactory $timeFactory
) {
$this->httpClient = $httpClientService->newClient();
$this->jobList = $jobList;
@ -107,6 +113,7 @@ class RequestSharedSecret extends Job {
$this->logger = $logger;
$this->ocsDiscoveryService = $ocsDiscoveryService;
$this->trustedServers = $trustedServers;
$this->timeFactory = $timeFactory;
}
@ -126,7 +133,7 @@ class RequestSharedSecret extends Job {
$jobList->remove($this, $this->argument);
if ($this->retainJob) {
$this->reAddJob($jobList, $this->argument);
$this->reAddJob($this->argument);
}
}
@ -143,8 +150,8 @@ class RequestSharedSecret extends Job {
protected function run($argument) {
$target = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : time();
$currentTime = time();
$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
$currentTime = $this->timeFactory->getTime();
$source = $this->urlGenerator->getAbsoluteURL('/');
$source = rtrim($source, '/');
$token = $argument['token'];
@ -208,16 +215,14 @@ class RequestSharedSecret extends Job {
/**
* re-add background job
*
* @param IJobList $jobList
* @param array $argument
*/
protected function reAddJob(IJobList $jobList, array $argument) {
protected function reAddJob(array $argument) {
$url = $argument['url'];
$created = isset($argument['created']) ? (int)$argument['created'] : time();
$created = isset($argument['created']) ? (int)$argument['created'] : $this->timeFactory->getTime();
$token = $argument['token'];
$jobList->add(
$this->jobList->add(
RequestSharedSecret::class,
[
'url' => $url,

View File

@ -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;
}
/**
@ -154,7 +161,7 @@ class OCSAuthAPIController extends OCSController{
[
'url' => $url,
'token' => $token,
'created' => $this->getTimestamp()
'created' => $this->timeFactory->getTime()
]
);
@ -202,9 +209,4 @@ class OCSAuthAPIController extends OCSController{
$storedToken = $this->dbHandler->getToken($url);
return hash_equals($storedToken, $token);
}
protected function getTimestamp() {
return time();
}
}

View File

@ -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;
}
/**
@ -112,7 +119,7 @@ class TrustedServers {
[
'url' => $url,
'token' => $token,
'created' => $this->getTimestamp()
'created' => $this->timeFactory->getTime()
]
);
}
@ -276,8 +283,4 @@ class TrustedServers {
return 'https://' . $url;
}
protected function getTimestamp() {
return time();
}
}

View File

@ -31,6 +31,7 @@ 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;
@ -75,6 +76,9 @@ class GetSharedSecretTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject|IDiscoveryService */
private $discoverService;
/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
private $timeFactory;
/** @var GetSharedSecret */
private $getSharedSecret;
@ -92,6 +96,7 @@ 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);
@ -103,7 +108,8 @@ class GetSharedSecretTest extends TestCase {
$this->trustedServers,
$this->logger,
$this->dbHandler,
$this->discoverService
$this->discoverService,
$this->timeFactory
);
}
@ -124,10 +130,11 @@ class GetSharedSecretTest extends TestCase {
$this->trustedServers,
$this->logger,
$this->dbHandler,
$this->discoverService
$this->discoverService,
$this->timeFactory
]
)->setMethods(['parentExecute', 'reAddJob'])->getMock();
$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url']]);
)->setMethods(['parentExecute'])->getMock();
$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
$this->trustedServers->expects($this->once())->method('isTrustedServer')
->with('url')->willReturn($isTrustedServer);
@ -138,11 +145,22 @@ class GetSharedSecretTest extends TestCase {
}
$this->invokePrivate($getSharedSecret, 'retainJob', [$retainBackgroundJob]);
$this->jobList->expects($this->once())->method('remove');
if ($retainBackgroundJob) {
$getSharedSecret->expects($this->once())->method('reAddJob');
} else {
$getSharedSecret->expects($this->never())->method('reAddJob');
$this->timeFactory->method('getTime')->willReturn(42);
if ($retainBackgroundJob) {
$this->jobList->expects($this->once())
->method('add')
->with(
GetSharedSecret::class,
[
'url' => 'url',
'token' => 'token',
'created' => 42,
]
);
} else {
$this->jobList->expects($this->never())->method('add');
}
$getSharedSecret->execute($this->jobList);
@ -163,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')
@ -216,7 +236,6 @@ class GetSharedSecretTest extends TestCase {
} else {
$this->assertFalse($this->invokePrivate($this->getSharedSecret, 'retainJob'));
}
}
public function dataTestRun() {
@ -227,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]);
}
}

View File

@ -29,6 +29,7 @@ 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;
@ -67,6 +68,9 @@ class RequestSharedSecretTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
private $logger;
/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
private $timeFactory;
/** @var RequestSharedSecret */
private $requestSharedSecret;
@ -84,6 +88,7 @@ class RequestSharedSecretTest extends TestCase {
$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);
@ -95,7 +100,8 @@ class RequestSharedSecretTest extends TestCase {
$this->trustedServers,
$this->dbHandler,
$this->discoveryService,
$this->logger
$this->logger,
$this->timeFactory
);
}
@ -116,10 +122,11 @@ class RequestSharedSecretTest extends TestCase {
$this->trustedServers,
$this->dbHandler,
$this->discoveryService,
$this->logger
$this->logger,
$this->timeFactory
]
)->setMethods(['parentExecute', 'reAddJob'])->getMock();
$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url']]);
)->setMethods(['parentExecute'])->getMock();
$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url', 'token' => 'token']]);
$this->trustedServers->expects($this->once())->method('isTrustedServer')
->with('url')->willReturn($isTrustedServer);
@ -130,10 +137,22 @@ class RequestSharedSecretTest extends TestCase {
}
$this->invokePrivate($requestSharedSecret, 'retainJob', [$retainBackgroundJob]);
$this->jobList->expects($this->once())->method('remove');
$this->timeFactory->method('getTime')->willReturn(42);
if ($retainBackgroundJob) {
$requestSharedSecret->expects($this->once())->method('reAddJob');
$this->jobList->expects($this->once())
->method('add')
->with(
RequestSharedSecret::class,
[
'url' => 'url',
'token' => 'token',
'created' => 42,
]
);
} else {
$requestSharedSecret->expects($this->never())->method('reAddJob');
$this->jobList->expects($this->never())->method('add');
}
$requestSharedSecret->execute($this->jobList);
@ -161,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')
@ -209,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]);
}
}

View File

@ -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;
@ -56,6 +56,10 @@ class OCSAuthAPIControllerTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
private $logger;
/** @var \PHPUnit_Framework_MockObject_MockObject|ITimeFactory */
private $timeFactory;
/** @var OCSAuthAPIController */
private $ocsAuthApi;
@ -65,31 +69,28 @@ class OCSAuthAPIControllerTest extends TestCase {
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 = $this->getMockBuilder(OCSAuthAPIController::class)
->setConstructorArgs(
[
$this->ocsAuthApi = new OCSAuthAPIController(
'federation',
$this->request,
$this->secureRandom,
$this->jobList,
$this->trustedServers,
$this->dbHandler,
$this->logger
]
)->setMethods(['getTimestamp'])->getMock();
$this->logger,
$this->timeFactory
);
$this->ocsAuthApi->expects($this->any())->method('getTimestamp')->willReturn($this->currentTime);
$this->timeFactory->method('getTime')
->willReturn($this->currentTime);
}
@ -157,7 +158,8 @@ class OCSAuthAPIControllerTest extends TestCase {
$this->jobList,
$this->trustedServers,
$this->dbHandler,
$this->logger
$this->logger,
$this->timeFactory
]
)->setMethods(['isValidToken'])->getMock();

View File

@ -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,14 +120,16 @@ class TrustedServersTest extends TestCase {
$this->jobList,
$this->secureRandom,
$this->config,
$this->dispatcher
$this->dispatcher,
$this->timeFactory
]
)
->setMethods(['normalizeUrl', 'updateProtocol', 'getTimestamp'])
->setMethods(['normalizeUrl', 'updateProtocol'])
->getMock();
$trustedServers->expects($this->once())->method('updateProtocol')
->with('url')->willReturn('https://url');
$trustedServers->expects($this->any())->method('getTimestamp')->willReturn(1234567);
$this->timeFactory->method('getTime')
->willReturn(1234567);
$this->dbHandler->expects($this->once())->method('addServer')->with('https://url')
->willReturn($success);
@ -273,7 +281,8 @@ class TrustedServersTest extends TestCase {
$this->jobList,
$this->secureRandom,
$this->config,
$this->dispatcher
$this->dispatcher,
$this->timeFactory
]
)
->setMethods(['checkOwnCloudVersion'])