From 16ff207465335b624e67b9a9d0dae00ef23cd45c Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Fri, 19 Aug 2016 14:25:59 +0200
Subject: [PATCH] Move OCSAuthAPI to AppFramework
* Convert class
* Convert routes
* Convert tests
---
apps/federation/appinfo/routes.php | 16 ++++-
.../OCSAuthAPIController.php} | 44 ++++++++-----
.../OCSAuthAPIControllerTest.php} | 65 ++++++++++---------
3 files changed, 76 insertions(+), 49 deletions(-)
rename apps/federation/lib/{API/OCSAuthAPI.php => Controller/OCSAuthAPIController.php} (85%)
rename apps/federation/tests/{API/OCSAuthAPITest.php => Controller/OCSAuthAPIControllerTest.php} (79%)
diff --git a/apps/federation/appinfo/routes.php b/apps/federation/appinfo/routes.php
index 03acc60c682..b9515812a01 100644
--- a/apps/federation/appinfo/routes.php
+++ b/apps/federation/appinfo/routes.php
@@ -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();
diff --git a/apps/federation/lib/API/OCSAuthAPI.php b/apps/federation/lib/Controller/OCSAuthAPIController.php
similarity index 85%
rename from apps/federation/lib/API/OCSAuthAPI.php
rename to apps/federation/lib/Controller/OCSAuthAPIController.php
index a22de155d4c..68e0f8b271e 100644
--- a/apps/federation/lib/API/OCSAuthAPI.php
+++ b/apps/federation/lib/Controller/OCSAuthAPIController.php
@@ -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,9 +91,13 @@ class OCSAuthAPI {
}
/**
+ * @NoCSRFRequired
+ * @PublicPage
+ *
* request received to ask remote server for a shared secret
*
- * @return \OC_OCS_Result
+ * @return Http\DataResponse
+ * @throws OCSForbiddenException
*/
public function requestSharedSecret() {
@@ -100,7 +106,7 @@ class OCSAuthAPI {
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 +117,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,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
*
- * @return \OC_OCS_Result
+ * @return Http\DataResponse
+ * @throws OCSForbiddenException
*/
public function getSharedSecret() {
@@ -147,7 +156,7 @@ class OCSAuthAPI {
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 +165,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 +174,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) {
diff --git a/apps/federation/tests/API/OCSAuthAPITest.php b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
similarity index 79%
rename from apps/federation/tests/API/OCSAuthAPITest.php
rename to apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
index 7861e917ff8..ba10ce57c30 100644
--- a/apps/federation/tests/API/OCSAuthAPITest.php
+++ b/apps/federation/tests/Controller/OCSAuthAPIControllerTest.php
@@ -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,9 +91,9 @@ 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';
@@ -103,7 +105,7 @@ class OCSAuthAPITest extends TestCase {
$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 +115,19 @@ class OCSAuthAPITest extends TestCase {
$this->jobList->expects($this->never())->method('remove');
}
- $result = $this->ocsAuthApi->requestSharedSecret();
- $this->assertSame($expected, $result->getStatusCode());
+ try {
+ $result = $this->ocsAuthApi->requestSharedSecret();
+ $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,9 +136,9 @@ 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';
@@ -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(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 +166,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 +180,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();
+ $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],
];
}
--
GitLab