aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--apps/federatedfilesharing/lib/AddressHandler.php16
-rw-r--r--apps/federatedfilesharing/lib/Notifications.php59
-rw-r--r--apps/federatedfilesharing/tests/AddressHandlerTest.php20
-rw-r--r--apps/federatedfilesharing/tests/NotificationsTest.php9
-rw-r--r--lib/private/Security/CSRF/CsrfToken.php6
-rw-r--r--tests/lib/Security/CSRF/CsrfTokenManagerTest.php8
-rw-r--r--tests/lib/Security/CSRF/CsrfTokenTest.php8
7 files changed, 81 insertions, 45 deletions
diff --git a/apps/federatedfilesharing/lib/AddressHandler.php b/apps/federatedfilesharing/lib/AddressHandler.php
index 3c178b813e4..5fc41c2c804 100644
--- a/apps/federatedfilesharing/lib/AddressHandler.php
+++ b/apps/federatedfilesharing/lib/AddressHandler.php
@@ -161,6 +161,22 @@ class AddressHandler {
}
/**
+ * check if the url contain the protocol (http or https)
+ *
+ * @param string $url
+ * @return bool
+ */
+ public function urlContainProtocol($url) {
+ if (strpos($url, 'https://') === 0 ||
+ strpos($url, 'http://') === 0) {
+
+ return true;
+ }
+
+ return false;
+ }
+
+ /**
* Strips away a potential file names and trailing slashes:
* - http://localhost
* - http://localhost/
diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php
index 074991c43f8..8110b4915da 100644
--- a/apps/federatedfilesharing/lib/Notifications.php
+++ b/apps/federatedfilesharing/lib/Notifications.php
@@ -84,7 +84,6 @@ class Notifications {
list($user, $remote) = $this->addressHandler->splitUserRemote($shareWith);
if ($user && $remote) {
- $url = $remote;
$local = $this->addressHandler->generateRemoteURL();
$fields = array(
@@ -99,8 +98,7 @@ class Notifications {
'remote' => $local,
);
- $url = $this->addressHandler->removeProtocolFromUrl($url);
- $result = $this->tryHttpPostToShareEndpoint($url, '', $fields);
+ $result = $this->tryHttpPostToShareEndpoint($remote, '', $fields);
$status = json_decode($result['result'], true);
if ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200)) {
@@ -135,8 +133,7 @@ class Notifications {
'remoteId' => $shareId
);
- $url = $this->addressHandler->removeProtocolFromUrl($remote);
- $result = $this->tryHttpPostToShareEndpoint(rtrim($url, '/'), '/' . $id . '/reshare', $fields);
+ $result = $this->tryHttpPostToShareEndpoint(rtrim($remote, '/'), '/' . $id . '/reshare', $fields);
$status = json_decode($result['result'], true);
$httpRequestSuccessful = $result['success'];
@@ -193,7 +190,7 @@ class Notifications {
/**
* forward accept reShare to remote server
- *
+ *
* @param string $remote
* @param int $remoteId
* @param string $token
@@ -231,8 +228,7 @@ class Notifications {
$fields[$key] = $value;
}
- $url = $this->addressHandler->removeProtocolFromUrl($remote);
- $result = $this->tryHttpPostToShareEndpoint(rtrim($url, '/'), '/' . $remoteId . '/' . $action, $fields);
+ $result = $this->tryHttpPostToShareEndpoint(rtrim($remote, '/'), '/' . $remoteId . '/' . $action, $fields);
$status = json_decode($result['result'], true);
if ($result['success'] &&
@@ -270,7 +266,8 @@ class Notifications {
}
/**
- * try http post first with https and then with http as a fallback
+ * try http post with the given protocol, if no protocol is given we pick
+ * the secure one (https)
*
* @param string $remoteDomain
* @param string $urlSuffix
@@ -280,33 +277,31 @@ class Notifications {
*/
protected function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) {
$client = $this->httpClientService->newClient();
- $protocol = 'https://';
+
+ if ($this->addressHandler->urlContainProtocol($remoteDomain) === false) {
+ $remoteDomain = 'https://' . $remoteDomain;
+ }
+
$result = [
'success' => false,
'result' => '',
];
- $try = 0;
-
- while ($result['success'] === false && $try < 2) {
- $endpoint = $this->discoveryManager->getShareEndpoint($protocol . $remoteDomain);
- try {
- $response = $client->post($protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, [
- 'body' => $fields,
- 'timeout' => 10,
- 'connect_timeout' => 10,
- ]);
- $result['result'] = $response->getBody();
- $result['success'] = true;
- break;
- } catch (\Exception $e) {
- // if flat re-sharing is not supported by the remote server
- // we re-throw the exception and fall back to the old behaviour.
- // (flat re-shares has been introduced in Nextcloud 9.1)
- if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) {
- throw $e;
- }
- $try++;
- $protocol = 'http://';
+
+ $endpoint = $this->discoveryManager->getShareEndpoint($remoteDomain);
+ try {
+ $response = $client->post($remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, [
+ 'body' => $fields,
+ 'timeout' => 10,
+ 'connect_timeout' => 10,
+ ]);
+ $result['result'] = $response->getBody();
+ $result['success'] = true;
+ } catch (\Exception $e) {
+ // if flat re-sharing is not supported by the remote server
+ // we re-throw the exception and fall back to the old behaviour.
+ // (flat re-shares has been introduced in Nextcloud 9.1)
+ if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) {
+ throw $e;
}
}
diff --git a/apps/federatedfilesharing/tests/AddressHandlerTest.php b/apps/federatedfilesharing/tests/AddressHandlerTest.php
index c2e69fb2bd7..f62f3b62e03 100644
--- a/apps/federatedfilesharing/tests/AddressHandlerTest.php
+++ b/apps/federatedfilesharing/tests/AddressHandlerTest.php
@@ -178,6 +178,26 @@ class AddressHandlerTest extends \Test\TestCase {
}
/**
+ * @dataProvider dataTestUrlContainProtocol
+ *
+ * @param string $url
+ * @param bool $expectedResult
+ */
+ public function testUrlContainProtocol($url, $expectedResult) {
+ $result = $this->addressHandler->urlContainProtocol($url);
+ $this->assertSame($expectedResult, $result);
+ }
+
+ public function dataTestUrlContainProtocol() {
+ return [
+ ['http://nextcloud.com', true],
+ ['https://nextcloud.com', true],
+ ['nextcloud.com', false],
+ ['httpserver.com', false],
+ ];
+ }
+
+ /**
* @dataProvider dataTestFixRemoteUrl
*
* @param string $url
diff --git a/apps/federatedfilesharing/tests/NotificationsTest.php b/apps/federatedfilesharing/tests/NotificationsTest.php
index dbcb1ef4e87..a5f5c6bc078 100644
--- a/apps/federatedfilesharing/tests/NotificationsTest.php
+++ b/apps/federatedfilesharing/tests/NotificationsTest.php
@@ -58,7 +58,7 @@ class NotificationsTest extends \Test\TestCase {
/**
* get instance of Notifications class
- *
+ *
* @param array $mockedMethods methods which should be mocked
* @return Notifications | \PHPUnit_Framework_MockObject_MockObject
*/
@@ -81,7 +81,7 @@ class NotificationsTest extends \Test\TestCase {
]
)->setMethods($mockedMethods)->getMock();
}
-
+
return $instance;
}
@@ -94,7 +94,7 @@ class NotificationsTest extends \Test\TestCase {
* @param bool $expected
*/
public function testSendUpdateToRemote($try, $httpRequestResult, $expected) {
- $remote = 'remote';
+ $remote = 'http://remote';
$id = 42;
$timestamp = 63576;
$token = 'token';
@@ -106,9 +106,6 @@ class NotificationsTest extends \Test\TestCase {
->with($remote, '/'.$id.'/unshare', ['token' => $token, 'data1Key' => 'data1Value'])
->willReturn($httpRequestResult);
- $this->addressHandler->expects($this->once())->method('removeProtocolFromUrl')
- ->with($remote)->willReturn($remote);
-
// only add background job on first try
if ($try === 0 && $expected === false) {
$this->jobList->expects($this->once())->method('add')
diff --git a/lib/private/Security/CSRF/CsrfToken.php b/lib/private/Security/CSRF/CsrfToken.php
index dce9a83b727..e9bdf5b5204 100644
--- a/lib/private/Security/CSRF/CsrfToken.php
+++ b/lib/private/Security/CSRF/CsrfToken.php
@@ -51,8 +51,8 @@ class CsrfToken {
*/
public function getEncryptedValue() {
if($this->encryptedValue === '') {
- $sharedSecret = base64_encode(random_bytes(strlen($this->value)));
- $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . $sharedSecret;
+ $sharedSecret = random_bytes(strlen($this->value));
+ $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . base64_encode($sharedSecret);
}
return $this->encryptedValue;
@@ -71,6 +71,6 @@ class CsrfToken {
}
$obfuscatedToken = $token[0];
$secret = $token[1];
- return base64_decode($obfuscatedToken) ^ $secret;
+ return base64_decode($obfuscatedToken) ^ base64_decode($secret);
}
}
diff --git a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
index 6f7842fdfd9..f9dd8127e5a 100644
--- a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
+++ b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
@@ -137,15 +137,19 @@ class CsrfTokenManagerTest extends \Test\TestCase {
}
public function testIsTokenValidWithValidToken() {
+ $a = 'abc';
+ $b = 'def';
+ $xorB64 = 'BQcF';
+ $tokenVal = sprintf('%s:%s', $xorB64, base64_encode($a));
$this->storageInterface
->expects($this->once())
->method('hasToken')
->willReturn(true);
- $token = new \OC\Security\CSRF\CsrfToken('XlQhHjgWCgBXAEI0Khl+IQEiCXN2LUcDHAQTQAc1HQs=:qgkUlg8l3m8WnkOG4XM9Az33pAt1vSVMx4hcJFsxdqc=');
+ $token = new \OC\Security\CSRF\CsrfToken($tokenVal);
$this->storageInterface
->expects($this->once())
->method('getToken')
- ->willReturn('/3JKTq2ldmzcDr1f5zDJ7Wt0lEgqqfKF');
+ ->willReturn($b);
$this->assertSame(true, $this->csrfTokenManager->isTokenValid($token));
}
diff --git a/tests/lib/Security/CSRF/CsrfTokenTest.php b/tests/lib/Security/CSRF/CsrfTokenTest.php
index d19d1de916c..fbb92cd315a 100644
--- a/tests/lib/Security/CSRF/CsrfTokenTest.php
+++ b/tests/lib/Security/CSRF/CsrfTokenTest.php
@@ -36,7 +36,11 @@ class CsrfTokenTest extends \Test\TestCase {
}
public function testGetDecryptedValue() {
- $csrfToken = new \OC\Security\CSRF\CsrfToken('XlQhHjgWCgBXAEI0Khl+IQEiCXN2LUcDHAQTQAc1HQs=:qgkUlg8l3m8WnkOG4XM9Az33pAt1vSVMx4hcJFsxdqc=');
- $this->assertSame('/3JKTq2ldmzcDr1f5zDJ7Wt0lEgqqfKF', $csrfToken->getDecryptedValue());
+ $a = 'abc';
+ $b = 'def';
+ $xorB64 = 'BQcF';
+ $tokenVal = sprintf('%s:%s', $xorB64, base64_encode($a));
+ $csrfToken = new \OC\Security\CSRF\CsrfToken($tokenVal);
+ $this->assertSame($b, $csrfToken->getDecryptedValue());
}
}