Move OCSAuthAPI to AppFramework

* Convert class
* Convert routes
* Convert tests
This commit is contained in:
Roeland Jago Douma 2016-08-19 14:25:59 +02:00
parent b580c3664d
commit 16ff207465
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
3 changed files with 76 additions and 49 deletions

View File

@ -41,8 +41,18 @@ $application->registerRoutes(
'url' => '/auto-add-servers', 'url' => '/auto-add-servers',
'verb' => 'POST' '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();

View File

@ -25,11 +25,13 @@
*/ */
namespace OCA\Federation\API; namespace OCA\Federation\Controller;
use OCA\Federation\DbHandler; use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers; use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http; use OCP\AppFramework\Http;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCSController;
use OCP\BackgroundJob\IJobList; use OCP\BackgroundJob\IJobList;
use OCP\ILogger; use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
@ -40,12 +42,9 @@ use OCP\Security\ISecureRandom;
* *
* OCS API end-points to exchange shared secret between two connected ownClouds * OCS API end-points to exchange shared secret between two connected ownClouds
* *
* @package OCA\Federation\API * @package OCA\Federation\Controller
*/ */
class OCSAuthAPI { class OCSAuthAPIController extends OCSController{
/** @var IRequest */
private $request;
/** @var ISecureRandom */ /** @var ISecureRandom */
private $secureRandom; private $secureRandom;
@ -65,6 +64,7 @@ class OCSAuthAPI {
/** /**
* OCSAuthAPI constructor. * OCSAuthAPI constructor.
* *
* @param string $appName
* @param IRequest $request * @param IRequest $request
* @param ISecureRandom $secureRandom * @param ISecureRandom $secureRandom
* @param IJobList $jobList * @param IJobList $jobList
@ -73,6 +73,7 @@ class OCSAuthAPI {
* @param ILogger $logger * @param ILogger $logger
*/ */
public function __construct( public function __construct(
$appName,
IRequest $request, IRequest $request,
ISecureRandom $secureRandom, ISecureRandom $secureRandom,
IJobList $jobList, IJobList $jobList,
@ -80,7 +81,8 @@ class OCSAuthAPI {
DbHandler $dbHandler, DbHandler $dbHandler,
ILogger $logger ILogger $logger
) { ) {
$this->request = $request; parent::__construct($appName, $request);
$this->secureRandom = $secureRandom; $this->secureRandom = $secureRandom;
$this->jobList = $jobList; $this->jobList = $jobList;
$this->trustedServers = $trustedServers; $this->trustedServers = $trustedServers;
@ -89,9 +91,13 @@ class OCSAuthAPI {
} }
/** /**
* @NoCSRFRequired
* @PublicPage
*
* request received to ask remote server for a shared secret * request received to ask remote server for a shared secret
* *
* @return \OC_OCS_Result * @return Http\DataResponse
* @throws OCSForbiddenException
*/ */
public function requestSharedSecret() { public function requestSharedSecret() {
@ -100,7 +106,7 @@ class OCSAuthAPI {
if ($this->trustedServers->isTrustedServer($url) === false) { if ($this->trustedServers->isTrustedServer($url) === false) {
$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']); $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 // if both server initiated the exchange of the shared secret the greater
@ -111,7 +117,7 @@ class OCSAuthAPI {
'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.', 'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.',
['app' => 'federation'] ['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 // we ask for the shared secret so we no longer have to ask the other server
@ -131,14 +137,17 @@ class OCSAuthAPI {
] ]
); );
return new \OC_OCS_Result(null, Http::STATUS_OK); return new Http\DataResponse();
} }
/** /**
* @NoCSRFRequired
* @PublicPage
*
* create shared secret and return it * create shared secret and return it
* *
* @return \OC_OCS_Result * @return Http\DataResponse
* @throws OCSForbiddenException
*/ */
public function getSharedSecret() { public function getSharedSecret() {
@ -147,7 +156,7 @@ class OCSAuthAPI {
if ($this->trustedServers->isTrustedServer($url) === false) { if ($this->trustedServers->isTrustedServer($url) === false) {
$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']); $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) { if ($this->isValidToken($url, $token) === false) {
@ -156,7 +165,7 @@ class OCSAuthAPI {
'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret', 'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret',
['app' => 'federation'] ['app' => 'federation']
); );
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN); throw new OCSForbiddenException();
} }
$sharedSecret = $this->secureRandom->generate(32); $sharedSecret = $this->secureRandom->generate(32);
@ -165,8 +174,9 @@ class OCSAuthAPI {
// reset token after the exchange of the shared secret was successful // reset token after the exchange of the shared secret was successful
$this->dbHandler->addToken($url, ''); $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) { protected function isValidToken($url, $token) {

View File

@ -22,20 +22,21 @@
*/ */
namespace OCA\Federation\Tests\API; namespace OCA\Federation\Tests\Controller;
use OC\BackgroundJob\JobList; use OC\BackgroundJob\JobList;
use OCA\Federation\API\OCSAuthAPI; use OCA\Federation\Controller\OCSAuthAPIController;
use OCA\Federation\DbHandler; use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers; use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http; use OCP\AppFramework\Http;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\ILogger; use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use Test\TestCase; use Test\TestCase;
class OCSAuthAPITest extends TestCase { class OCSAuthAPIControllerTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject | IRequest */ /** @var \PHPUnit_Framework_MockObject_MockObject | IRequest */
private $request; private $request;
@ -55,14 +56,14 @@ class OCSAuthAPITest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */ /** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
private $logger; private $logger;
/** @var OCSAuthApi */ /** @var OCSAuthAPIController */
private $ocsAuthApi; private $ocsAuthApi;
public function setUp() { public function setUp() {
parent::setUp(); parent::setUp();
$this->request = $this->getMock('OCP\IRequest'); $this->request = $this->getMockBuilder('OCP\IRequest')->getMock();
$this->secureRandom = $this->getMock('OCP\Security\ISecureRandom'); $this->secureRandom = $this->getMockBuilder('OCP\Security\ISecureRandom')->getMock();
$this->trustedServers = $this->getMockBuilder('OCA\Federation\TrustedServers') $this->trustedServers = $this->getMockBuilder('OCA\Federation\TrustedServers')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler') $this->dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')
@ -72,7 +73,8 @@ class OCSAuthAPITest extends TestCase {
$this->logger = $this->getMockBuilder('OCP\ILogger') $this->logger = $this->getMockBuilder('OCP\ILogger')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->ocsAuthApi = new OCSAuthAPI( $this->ocsAuthApi = new OCSAuthAPIController(
'federation',
$this->request, $this->request,
$this->secureRandom, $this->secureRandom,
$this->jobList, $this->jobList,
@ -89,9 +91,9 @@ class OCSAuthAPITest extends TestCase {
* @param string $token * @param string $token
* @param string $localToken * @param string $localToken
* @param bool $isTrustedServer * @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'; $url = 'url';
@ -103,7 +105,7 @@ class OCSAuthAPITest extends TestCase {
$this->dbHandler->expects($this->any()) $this->dbHandler->expects($this->any())
->method('getToken')->with($url)->willReturn($localToken); ->method('getToken')->with($url)->willReturn($localToken);
if ($expected === Http::STATUS_OK) { if ($ok) {
$this->jobList->expects($this->once())->method('add') $this->jobList->expects($this->once())->method('add')
->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token]); ->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token]);
$this->jobList->expects($this->once())->method('remove') $this->jobList->expects($this->once())->method('remove')
@ -113,15 +115,19 @@ class OCSAuthAPITest extends TestCase {
$this->jobList->expects($this->never())->method('remove'); $this->jobList->expects($this->never())->method('remove');
} }
try {
$result = $this->ocsAuthApi->requestSharedSecret(); $result = $this->ocsAuthApi->requestSharedSecret();
$this->assertSame($expected, $result->getStatusCode()); $this->assertTrue($ok);
} catch (OCSForbiddenException $e) {
$this->assertFalse($ok);
}
} }
public function dataTestRequestSharedSecret() { public function dataTestRequestSharedSecret() {
return [ return [
['token2', 'token1', true, Http::STATUS_OK], ['token2', 'token1', true, true],
['token1', 'token2', false, Http::STATUS_FORBIDDEN], ['token1', 'token2', false, false],
['token1', 'token2', true, Http::STATUS_FORBIDDEN], ['token1', 'token2', true, false],
]; ];
} }
@ -130,9 +136,9 @@ class OCSAuthAPITest extends TestCase {
* *
* @param bool $isTrustedServer * @param bool $isTrustedServer
* @param bool $isValidToken * @param bool $isValidToken
* @param int $expected * @param bool $ok
*/ */
public function testGetSharedSecret($isTrustedServer, $isValidToken, $expected) { public function testGetSharedSecret($isTrustedServer, $isValidToken, $ok) {
$url = 'url'; $url = 'url';
$token = 'token'; $token = 'token';
@ -140,10 +146,11 @@ class OCSAuthAPITest extends TestCase {
$this->request->expects($this->at(0))->method('getParam')->with('url')->willReturn($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->request->expects($this->at(1))->method('getParam')->with('token')->willReturn($token);
/** @var OCSAuthAPI | \PHPUnit_Framework_MockObject_MockObject $ocsAuthApi */ /** @var OCSAuthAPIController | \PHPUnit_Framework_MockObject_MockObject $ocsAuthApi */
$ocsAuthApi = $this->getMockBuilder('OCA\Federation\API\OCSAuthAPI') $ocsAuthApi = $this->getMockBuilder('OCA\Federation\Controller\OCSAuthAPIController')
->setConstructorArgs( ->setConstructorArgs(
[ [
'federation',
$this->request, $this->request,
$this->secureRandom, $this->secureRandom,
$this->jobList, $this->jobList,
@ -159,7 +166,7 @@ class OCSAuthAPITest extends TestCase {
$ocsAuthApi->expects($this->any()) $ocsAuthApi->expects($this->any())
->method('isValidToken')->with($url, $token)->willReturn($isValidToken); ->method('isValidToken')->with($url, $token)->willReturn($isValidToken);
if($expected === Http::STATUS_OK) { if($ok) {
$this->secureRandom->expects($this->once())->method('generate')->with(32) $this->secureRandom->expects($this->once())->method('generate')->with(32)
->willReturn('secret'); ->willReturn('secret');
$this->trustedServers->expects($this->once()) $this->trustedServers->expects($this->once())
@ -173,22 +180,22 @@ class OCSAuthAPITest extends TestCase {
$this->dbHandler->expects($this->never())->method('addToken'); $this->dbHandler->expects($this->never())->method('addToken');
} }
try {
$result = $ocsAuthApi->getSharedSecret(); $result = $ocsAuthApi->getSharedSecret();
$this->assertTrue($ok);
$this->assertSame($expected, $result->getStatusCode());
if ($expected === Http::STATUS_OK) {
$data = $result->getData(); $data = $result->getData();
$this->assertSame('secret', $data['sharedSecret']); $this->assertSame('secret', $data['sharedSecret']);
} catch (OCSForbiddenException $e) {
$this->assertFalse($ok);
} }
} }
public function dataTestGetSharedSecret() { public function dataTestGetSharedSecret() {
return [ return [
[true, true, Http::STATUS_OK], [true, true, true],
[false, true, Http::STATUS_FORBIDDEN], [false, true, false],
[true, false, Http::STATUS_FORBIDDEN], [true, false, false],
[false, false, Http::STATUS_FORBIDDEN], [false, false, false],
]; ];
} }