Merge pull request #988 from nextcloud/ocs_federation
Move OCSAuthAPI to AppFramework
This commit is contained in:
Morris Jobke
GitHub
commit
dd2482e2c2
|
|
@ -41,8 +41,18 @@ $application->registerRoutes(
|
|||
'url' => '/auto-add-servers',
|
||||
'verb' => 'POST'
|
||||
],
|
||||
]
|
||||
],
|
||||
'ocs' => [
|
||||
[
|
||||
'name' => 'OCSAuthAPI#getSharedSecret',
|
||||
'url' => '/api/v1/shared-secret',
|
||||
'verb' => 'GET',
|
||||
],
|
||||
[
|
||||
'name' => 'OCSAuthAPI#requestSharedSecret',
|
||||
'url' => '/api/v1/request-shared-secret',
|
||||
'verb' => 'POST',
|
||||
],
|
||||
],
|
||||
]
|
||||
);
|
||||
|
||||
$application->registerOCSApi();
|
||||
|
|
|
|||
|
|
@ -25,11 +25,13 @@
|
|||
*/
|
||||
|
||||
|
||||
namespace OCA\Federation\API;
|
||||
namespace OCA\Federation\Controller;
|
||||
|
||||
use OCA\Federation\DbHandler;
|
||||
use OCA\Federation\TrustedServers;
|
||||
use OCP\AppFramework\Http;
|
||||
use OCP\AppFramework\OCS\OCSForbiddenException;
|
||||
use OCP\AppFramework\OCSController;
|
||||
use OCP\BackgroundJob\IJobList;
|
||||
use OCP\ILogger;
|
||||
use OCP\IRequest;
|
||||
|
|
@ -40,12 +42,9 @@ use OCP\Security\ISecureRandom;
|
|||
*
|
||||
* OCS API end-points to exchange shared secret between two connected ownClouds
|
||||
*
|
||||
* @package OCA\Federation\API
|
||||
* @package OCA\Federation\Controller
|
||||
*/
|
||||
class OCSAuthAPI {
|
||||
|
||||
/** @var IRequest */
|
||||
private $request;
|
||||
class OCSAuthAPIController extends OCSController{
|
||||
|
||||
/** @var ISecureRandom */
|
||||
private $secureRandom;
|
||||
|
|
@ -65,6 +64,7 @@ class OCSAuthAPI {
|
|||
/**
|
||||
* OCSAuthAPI constructor.
|
||||
*
|
||||
* @param string $appName
|
||||
* @param IRequest $request
|
||||
* @param ISecureRandom $secureRandom
|
||||
* @param IJobList $jobList
|
||||
|
|
@ -73,6 +73,7 @@ class OCSAuthAPI {
|
|||
* @param ILogger $logger
|
||||
*/
|
||||
public function __construct(
|
||||
$appName,
|
||||
IRequest $request,
|
||||
ISecureRandom $secureRandom,
|
||||
IJobList $jobList,
|
||||
|
|
@ -80,7 +81,8 @@ class OCSAuthAPI {
|
|||
DbHandler $dbHandler,
|
||||
ILogger $logger
|
||||
) {
|
||||
$this->request = $request;
|
||||
parent::__construct($appName, $request);
|
||||
|
||||
$this->secureRandom = $secureRandom;
|
||||
$this->jobList = $jobList;
|
||||
$this->trustedServers = $trustedServers;
|
||||
|
|
@ -89,18 +91,20 @@ class OCSAuthAPI {
|
|||
}
|
||||
|
||||
/**
|
||||
* @NoCSRFRequired
|
||||
* @PublicPage
|
||||
*
|
||||
* request received to ask remote server for a shared secret
|
||||
*
|
||||
* @return \OC_OCS_Result
|
||||
* @param string $url
|
||||
* @param string $token
|
||||
* @return Http\DataResponse
|
||||
* @throws OCSForbiddenException
|
||||
*/
|
||||
public function requestSharedSecret() {
|
||||
|
||||
$url = $this->request->getParam('url');
|
||||
$token = $this->request->getParam('token');
|
||||
|
||||
public function requestSharedSecret($url, $token) {
|
||||
if ($this->trustedServers->isTrustedServer($url) === false) {
|
||||
$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']);
|
||||
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||
throw new OCSForbiddenException();
|
||||
}
|
||||
|
||||
// if both server initiated the exchange of the shared secret the greater
|
||||
|
|
@ -111,7 +115,7 @@ class OCSAuthAPI {
|
|||
'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.',
|
||||
['app' => 'federation']
|
||||
);
|
||||
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||
throw new OCSForbiddenException();
|
||||
}
|
||||
|
||||
// we ask for the shared secret so we no longer have to ask the other server
|
||||
|
|
@ -131,23 +135,24 @@ class OCSAuthAPI {
|
|||
]
|
||||
);
|
||||
|
||||
return new \OC_OCS_Result(null, Http::STATUS_OK);
|
||||
|
||||
return new Http\DataResponse();
|
||||
}
|
||||
|
||||
/**
|
||||
* @NoCSRFRequired
|
||||
* @PublicPage
|
||||
*
|
||||
* create shared secret and return it
|
||||
*
|
||||
* @return \OC_OCS_Result
|
||||
* @param string $url
|
||||
* @param string $token
|
||||
* @return Http\DataResponse
|
||||
* @throws OCSForbiddenException
|
||||
*/
|
||||
public function getSharedSecret() {
|
||||
|
||||
$url = $this->request->getParam('url');
|
||||
$token = $this->request->getParam('token');
|
||||
|
||||
public function getSharedSecret($url, $token) {
|
||||
if ($this->trustedServers->isTrustedServer($url) === false) {
|
||||
$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']);
|
||||
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||
throw new OCSForbiddenException();
|
||||
}
|
||||
|
||||
if ($this->isValidToken($url, $token) === false) {
|
||||
|
|
@ -156,7 +161,7 @@ class OCSAuthAPI {
|
|||
'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret',
|
||||
['app' => 'federation']
|
||||
);
|
||||
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||
throw new OCSForbiddenException();
|
||||
}
|
||||
|
||||
$sharedSecret = $this->secureRandom->generate(32);
|
||||
|
|
@ -165,8 +170,9 @@ class OCSAuthAPI {
|
|||
// reset token after the exchange of the shared secret was successful
|
||||
$this->dbHandler->addToken($url, '');
|
||||
|
||||
return new \OC_OCS_Result(['sharedSecret' => $sharedSecret], Http::STATUS_OK);
|
||||
|
||||
return new Http\DataResponse([
|
||||
'sharedSecret' => $sharedSecret
|
||||
]);
|
||||
}
|
||||
|
||||
protected function isValidToken($url, $token) {
|
||||
|
|
@ -22,20 +22,21 @@
|
|||
*/
|
||||
|
||||
|
||||
namespace OCA\Federation\Tests\API;
|
||||
namespace OCA\Federation\Tests\Controller;
|
||||
|
||||
|
||||
use OC\BackgroundJob\JobList;
|
||||
use OCA\Federation\API\OCSAuthAPI;
|
||||
use OCA\Federation\Controller\OCSAuthAPIController;
|
||||
use OCA\Federation\DbHandler;
|
||||
use OCA\Federation\TrustedServers;
|
||||
use OCP\AppFramework\Http;
|
||||
use OCP\AppFramework\OCS\OCSForbiddenException;
|
||||
use OCP\ILogger;
|
||||
use OCP\IRequest;
|
||||
use OCP\Security\ISecureRandom;
|
||||
use Test\TestCase;
|
||||
|
||||
class OCSAuthAPITest extends TestCase {
|
||||
class OCSAuthAPIControllerTest extends TestCase {
|
||||
|
||||
/** @var \PHPUnit_Framework_MockObject_MockObject | IRequest */
|
||||
private $request;
|
||||
|
|
@ -55,14 +56,14 @@ class OCSAuthAPITest extends TestCase {
|
|||
/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
|
||||
private $logger;
|
||||
|
||||
/** @var OCSAuthApi */
|
||||
/** @var OCSAuthAPIController */
|
||||
private $ocsAuthApi;
|
||||
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->request = $this->getMock('OCP\IRequest');
|
||||
$this->secureRandom = $this->getMock('OCP\Security\ISecureRandom');
|
||||
$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')
|
||||
|
|
@ -72,7 +73,8 @@ class OCSAuthAPITest extends TestCase {
|
|||
$this->logger = $this->getMockBuilder('OCP\ILogger')
|
||||
->disableOriginalConstructor()->getMock();
|
||||
|
||||
$this->ocsAuthApi = new OCSAuthAPI(
|
||||
$this->ocsAuthApi = new OCSAuthAPIController(
|
||||
'federation',
|
||||
$this->request,
|
||||
$this->secureRandom,
|
||||
$this->jobList,
|
||||
|
|
@ -89,21 +91,19 @@ class OCSAuthAPITest extends TestCase {
|
|||
* @param string $token
|
||||
* @param string $localToken
|
||||
* @param bool $isTrustedServer
|
||||
* @param int $expected
|
||||
* @param bool $ok
|
||||
*/
|
||||
public function testRequestSharedSecret($token, $localToken, $isTrustedServer, $expected) {
|
||||
public function testRequestSharedSecret($token, $localToken, $isTrustedServer, $ok) {
|
||||
|
||||
$url = 'url';
|
||||
|
||||
$this->request->expects($this->at(0))->method('getParam')->with('url')->willReturn($url);
|
||||
$this->request->expects($this->at(1))->method('getParam')->with('token')->willReturn($token);
|
||||
$this->trustedServers
|
||||
->expects($this->once())
|
||||
->method('isTrustedServer')->with($url)->willReturn($isTrustedServer);
|
||||
$this->dbHandler->expects($this->any())
|
||||
->method('getToken')->with($url)->willReturn($localToken);
|
||||
|
||||
if ($expected === Http::STATUS_OK) {
|
||||
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')
|
||||
|
|
@ -113,15 +113,19 @@ class OCSAuthAPITest extends TestCase {
|
|||
$this->jobList->expects($this->never())->method('remove');
|
||||
}
|
||||
|
||||
$result = $this->ocsAuthApi->requestSharedSecret();
|
||||
$this->assertSame($expected, $result->getStatusCode());
|
||||
try {
|
||||
$this->ocsAuthApi->requestSharedSecret($url, $token);
|
||||
$this->assertTrue($ok);
|
||||
} catch (OCSForbiddenException $e) {
|
||||
$this->assertFalse($ok);
|
||||
}
|
||||
}
|
||||
|
||||
public function dataTestRequestSharedSecret() {
|
||||
return [
|
||||
['token2', 'token1', true, Http::STATUS_OK],
|
||||
['token1', 'token2', false, Http::STATUS_FORBIDDEN],
|
||||
['token1', 'token2', true, Http::STATUS_FORBIDDEN],
|
||||
['token2', 'token1', true, true],
|
||||
['token1', 'token2', false, false],
|
||||
['token1', 'token2', true, false],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
@ -130,20 +134,18 @@ class OCSAuthAPITest extends TestCase {
|
|||
*
|
||||
* @param bool $isTrustedServer
|
||||
* @param bool $isValidToken
|
||||
* @param int $expected
|
||||
* @param bool $ok
|
||||
*/
|
||||
public function testGetSharedSecret($isTrustedServer, $isValidToken, $expected) {
|
||||
public function testGetSharedSecret($isTrustedServer, $isValidToken, $ok) {
|
||||
|
||||
$url = 'url';
|
||||
$token = 'token';
|
||||
|
||||
$this->request->expects($this->at(0))->method('getParam')->with('url')->willReturn($url);
|
||||
$this->request->expects($this->at(1))->method('getParam')->with('token')->willReturn($token);
|
||||
|
||||
/** @var OCSAuthAPI | \PHPUnit_Framework_MockObject_MockObject $ocsAuthApi */
|
||||
$ocsAuthApi = $this->getMockBuilder('OCA\Federation\API\OCSAuthAPI')
|
||||
/** @var OCSAuthAPIController | \PHPUnit_Framework_MockObject_MockObject $ocsAuthApi */
|
||||
$ocsAuthApi = $this->getMockBuilder('OCA\Federation\Controller\OCSAuthAPIController')
|
||||
->setConstructorArgs(
|
||||
[
|
||||
'federation',
|
||||
$this->request,
|
||||
$this->secureRandom,
|
||||
$this->jobList,
|
||||
|
|
@ -159,7 +161,7 @@ class OCSAuthAPITest extends TestCase {
|
|||
$ocsAuthApi->expects($this->any())
|
||||
->method('isValidToken')->with($url, $token)->willReturn($isValidToken);
|
||||
|
||||
if($expected === Http::STATUS_OK) {
|
||||
if($ok) {
|
||||
$this->secureRandom->expects($this->once())->method('generate')->with(32)
|
||||
->willReturn('secret');
|
||||
$this->trustedServers->expects($this->once())
|
||||
|
|
@ -173,22 +175,22 @@ class OCSAuthAPITest extends TestCase {
|
|||
$this->dbHandler->expects($this->never())->method('addToken');
|
||||
}
|
||||
|
||||
$result = $ocsAuthApi->getSharedSecret();
|
||||
|
||||
$this->assertSame($expected, $result->getStatusCode());
|
||||
|
||||
if ($expected === Http::STATUS_OK) {
|
||||
try {
|
||||
$result = $ocsAuthApi->getSharedSecret($url, $token);
|
||||
$this->assertTrue($ok);
|
||||
$data = $result->getData();
|
||||
$this->assertSame('secret', $data['sharedSecret']);
|
||||
} catch (OCSForbiddenException $e) {
|
||||
$this->assertFalse($ok);
|
||||
}
|
||||
}
|
||||
|
||||
public function dataTestGetSharedSecret() {
|
||||
return [
|
||||
[true, true, Http::STATUS_OK],
|
||||
[false, true, Http::STATUS_FORBIDDEN],
|
||||
[true, false, Http::STATUS_FORBIDDEN],
|
||||
[false, false, Http::STATUS_FORBIDDEN],
|
||||
[true, true, true],
|
||||
[false, true, false],
|
||||
[true, false, false],
|
||||
[false, false, false],
|
||||
];
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue