Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions apps/federation/lib/Controller/OCSAuthAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCA\Federation\BackgroundJob\RequestSharedSecret;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
Expand Down Expand Up @@ -109,6 +110,9 @@ public function requestSharedSecret(string $url, string $token): DataResponse {
$this->logger->info(
'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.'
);

$this->addRequestSharedSecretJobIfNeeded($url, $localToken);

throw new OCSForbiddenException();
}

Expand All @@ -124,6 +128,50 @@ public function requestSharedSecret(string $url, string $token): DataResponse {
return new DataResponse();
}

/**
* Adds a RequestSharedSecret job for the given URL if it does not exist.
*
* @param string $url the URL to add the job for.
* @param string $token the local token for the trusted server URL.
*/
private function addRequestSharedSecretJobIfNeeded(string $url, string $token) {
if ($this->hasRequestSharedSecretJobFor($url)) {
return;
}

$this->logger->info(
'No RequestSharedSecret job for ' . $url . ', a new one will be added to initiate the exchange of the shared secret.'
);

$this->jobList->add(
RequestSharedSecret::class,
[
'url' => $url,
'token' => $token,
'created' => $this->timeFactory->getTime()
]
);
}

/**
* Returns whether there is a RequestSharedSecret job for the given URL or
* not.
*
* Other arguments of the job are not taken into account.
*
* @param string $url the URL to check if there is a job for it.
* @return bool True if there is a job, false otherwise.
*/
private function hasRequestSharedSecretJobFor(string $url) {
foreach ($this->jobList->getJobsIterator(RequestSharedSecret::class, null, 0) as $job) {
if (is_array($job->getArgument()) && strcmp($job->getArgument()['url'], $url) === 0) {
return true;
}
}

return false;
}

/**
* Create shared secret and return it
*
Expand Down
39 changes: 35 additions & 4 deletions apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@

use OC\BackgroundJob\JobList;
use OCA\Federation\BackgroundJob\GetSharedSecret;
use OCA\Federation\BackgroundJob\RequestSharedSecret;
use OCA\Federation\Controller\OCSAuthAPIController;
use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJob;
use OCP\IRequest;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\ISecureRandom;
Expand Down Expand Up @@ -65,7 +67,7 @@ protected function setUp(): void {
}

#[\PHPUnit\Framework\Attributes\DataProvider('dataTestRequestSharedSecret')]
public function testRequestSharedSecret(string $token, string $localToken, bool $isTrustedServer, bool $ok): void {
public function testRequestSharedSecret(string $token, string $localToken, bool $isTrustedServer, ?bool $hasRequestSharedSecretJob, bool $ok): void {
$url = 'url';

$this->trustedServers
Expand All @@ -74,9 +76,37 @@ public function testRequestSharedSecret(string $token, string $localToken, bool
$this->dbHandler->expects($this->any())
->method('getToken')->with($url)->willReturn($localToken);

if (strcmp($token, $localToken) < 0 && $isTrustedServer) {
$this->jobList->expects($this->once())->method('getJobsIterator')
->with(RequestSharedSecret::class, null, 0)
->willReturnCallback(function() use ($hasRequestSharedSecretJob) {
$jobWithoutArguments = $this->createMock(IJob::class);
$jobWithoutArguments->method('getArgument')->willReturn(null);
$jobForAnotherServer = $this->createMock(IJob::class);
$jobForAnotherServer->method('getArgument')->willReturn(['url' => 'other url']);
$data = [
$jobWithoutArguments,
$jobForAnotherServer,
];

if ($hasRequestSharedSecretJob) {
$jobForSameServer = $this->createMock(IJob::class);
$jobForSameServer->method('getArgument')->willReturn(['url' => 'url']);
array_push($data, $jobForSameServer);
}

foreach ($data as $element) {
yield $element;
}
});
}

if ($ok) {
$this->jobList->expects($this->once())->method('add')
->with(GetSharedSecret::class, ['url' => $url, 'token' => $token, 'created' => $this->currentTime]);
} elseif (strcmp($token, $localToken) < 0 && $isTrustedServer && !$hasRequestSharedSecretJob) {
$this->jobList->expects($this->once())->method('add')
->with(RequestSharedSecret::class, ['url' => $url, 'token' => $localToken, 'created' => $this->currentTime]);
} else {
$this->jobList->expects($this->never())->method('add');
$this->jobList->expects($this->never())->method('remove');
Expand All @@ -98,9 +128,10 @@ public function testRequestSharedSecret(string $token, string $localToken, bool

public static function dataTestRequestSharedSecret(): array {
return [
['token2', 'token1', true, true],
['token1', 'token2', false, false],
['token1', 'token2', true, false],
['token2', 'token1', true, null, true],
['token1', 'token2', true, true, false],
['token1', 'token2', true, false, false],
['token1', 'token2', false, null, false],
];
}

Expand Down
Loading