Skip to content

Commit 846858b

Browse files
icewind1991icewind1991
authored
Merge pull request #689 from nextcloud/release-1.3.1
Release 1.3.1
2 parents 4e889f3 + f256965 commit 846858b

9 files changed

Lines changed: 123 additions & 37 deletions

File tree

‎appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<description><![CDATA[Push update support for desktop app.
1212
1313
Once the app is installed, the push binary needs to be setup. You can either use the setup wizard with `occ notify_push:setup` or see the [README](http://github.com/nextcloud/notify_push) for detailed setup instructions]]></description>
14-
<version>1.3.0</version>
14+
<version>1.3.1</version>
1515
<licence>agpl</licence>
1616
<author>Robin Appelman</author>
1717
<namespace>NotifyPush</namespace>

‎lib/Controller/TestController.php‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@
1010

1111
use OC\AppFramework\Http\Request;
1212
use OCA\NotifyPush\Queue\IQueue;
13-
use OCA\NotifyPush\Queue\RedisQueue;
1413
use OCP\App\IAppManager;
1514
use OCP\AppFramework\Controller;
15+
use OCP\AppFramework\Http;
1616
use OCP\AppFramework\Http\DataDisplayResponse;
1717
use OCP\AppFramework\Http\DataResponse;
18+
use OCP\AppFramework\Http\Response;
1819
use OCP\IAppConfig;
1920
use OCP\IRequest;
2021

@@ -33,10 +34,17 @@ public function __construct(
3334
* @PublicPage
3435
* @NoCSRFRequired
3536
*/
36-
public function cookie(): DataResponse {
37+
public function cookie(): Response {
3738
// starting with 32, the app config does some internal caching
3839
// that interferes with the quick set+get from this test.
3940
$this->appConfig->clearCache();
41+
42+
$expected = $this->queue->get('test-token');
43+
$token = $this->request->getHeader('token');
44+
if ($expected !== $token) {
45+
return new Response(Http::STATUS_FORBIDDEN);
46+
}
47+
4048
return new DataResponse($this->appConfig->getValueInt('notify_push', 'cookie'));
4149
}
4250

@@ -45,12 +53,16 @@ public function cookie(): DataResponse {
4553
* @PublicPage
4654
* @NoCSRFRequired
4755
*/
48-
public function remote(): DataDisplayResponse {
49-
if ($this->queue instanceof RedisQueue) {
50-
if ($this->request instanceof Request) {
51-
$this->queue->getConnection()->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for'));
52-
$this->queue->getConnection()->set('notify_push_remote', $this->request->server['REMOTE_ADDR']);
53-
}
56+
public function remote(): Response {
57+
$expected = $this->queue->get('test-token');
58+
$token = $this->request->getHeader('token');
59+
if ($expected !== $token) {
60+
return new Response(Http::STATUS_FORBIDDEN);
61+
}
62+
63+
if ($this->request instanceof Request) {
64+
$this->queue->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for'));
65+
$this->queue->set('notify_push_remote', $this->request->server['REMOTE_ADDR']);
5466
}
5567
return new DataDisplayResponse($this->request->getRemoteAddress());
5668
}
@@ -65,8 +77,6 @@ public function remote(): DataDisplayResponse {
6577
* @return void
6678
*/
6779
public function version(): void {
68-
if ($this->queue instanceof RedisQueue) {
69-
$this->queue->getConnection()->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push'));
70-
}
80+
$this->queue->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push'));
7181
}
7282
}

‎lib/Queue/IQueue.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,17 @@
1010

1111
interface IQueue {
1212
/**
13-
* @param string $channel
1413
* @param mixed $message
15-
* @return void
1614
*/
17-
public function push(string $channel, $message);
15+
public function push(string $channel, $message): void;
16+
17+
/**
18+
* @param mixed $value
19+
*/
20+
public function set(string $key, $value): void;
21+
22+
/**
23+
* @return mixed
24+
*/
25+
public function get(string $key);
1826
}

‎lib/Queue/NullQueue.php‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,19 @@
99
namespace OCA\NotifyPush\Queue;
1010

1111
class NullQueue implements IQueue {
12-
/**
13-
* @return void
14-
*/
15-
public function push(string $channel, $message) {
12+
#[\Override]
13+
public function push(string $channel, $message): void {
1614
// noop
1715
}
16+
17+
#[\Override]
18+
public function set(string $key, $value): void {
19+
// noop
20+
}
21+
22+
#[\Override]
23+
public function get(string $key) {
24+
return null;
25+
}
26+
1827
}

‎lib/Queue/RedisQueue.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ public function __construct($redis) {
1818
$this->redis = $redis;
1919
}
2020

21-
public function push(string $channel, $message) {
21+
#[\Override]
22+
public function push(string $channel, $message): void {
2223
$this->redis->publish($channel, json_encode($message));
2324
}
2425

@@ -28,4 +29,14 @@ public function push(string $channel, $message) {
2829
public function getConnection() {
2930
return $this->redis;
3031
}
32+
33+
#[\Override]
34+
public function set(string $key, $value): void {
35+
$this->redis->set($key, $value);
36+
}
37+
38+
#[\Override]
39+
public function get(string $key) {
40+
return $this->redis->get($key);
41+
}
3142
}

‎lib/SelfTest.php‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,22 @@
1212
use OCA\NotifyPush\Queue\IQueue;
1313
use OCA\NotifyPush\Queue\RedisQueue;
1414
use OCP\App\IAppManager;
15+
use OCP\Http\Client\IClient;
1516
use OCP\Http\Client\IClientService;
1617
use OCP\IAppConfig;
1718
use OCP\IConfig;
1819
use OCP\IDBConnection;
20+
use OCP\Security\ISecureRandom;
1921
use Symfony\Component\Console\Output\OutputInterface;
2022
use Symfony\Component\HttpFoundation\IpUtils;
2123

2224
class SelfTest {
2325
public const ERROR_OTHER = 1;
2426
public const ERROR_TRUSTED_PROXY = 2;
2527

26-
private $client;
27-
private $cookie;
28+
private IClient $client;
29+
private int $cookie;
30+
private string $token;
2831

2932
public function __construct(
3033
IClientService $clientService,
@@ -33,9 +36,15 @@ public function __construct(
3336
private IQueue $queue,
3437
private IDBConnection $connection,
3538
private IAppManager $appManager,
39+
private ISecureRandom $random,
3640
) {
3741
$this->client = $clientService->newClient();
3842
$this->cookie = rand(1, (int)pow(2, 30));
43+
$this->token = $this->random->generate(32);
44+
}
45+
46+
private function getHttpOpts(): array {
47+
return ['nextcloud' => ['allow_local_address' => true], 'verify' => false, 'headers' => ['token' => $this->token]];
3948
}
4049

4150
public function test(string $server, OutputInterface $output, bool $ignoreProxyError = false): int {
@@ -56,11 +65,12 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
5665
$output->writeln('<comment>🗴 push server URL is set to localhost, the push server will not be reachable from other machines</comment>');
5766
}
5867

68+
$this->queue->getConnection()->set('test-token', $this->token);
5969
$this->queue->push('notify_test_cookie', $this->cookie);
6070
$this->appConfig->setValueInt('notify_push', 'cookie', $this->cookie);
6171

6272
try {
63-
$retrievedCookie = (int)$this->client->get($server . '/test/cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
73+
$retrievedCookie = (int)$this->client->get($server . '/test/cookie', $this->getHttpOpts())->getBody();
6474
} catch (\Exception $e) {
6575
$msg = $e->getMessage();
6676
$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -80,7 +90,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
8090
// If no admin user was created during the installation, there are no oc_filecache and oc_mounts entries yet, so this check has to be skipped.
8191
if ($storageId !== null) {
8292
try {
83-
$retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
93+
$retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, $this->getHttpOpts())->getBody();
8494
} catch (\Exception $e) {
8595
$msg = $e->getMessage();
8696
$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -97,7 +107,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
97107

98108
// test if the push server can reach nextcloud by having it request the cookie
99109
try {
100-
$response = $this->client->get($server . '/test/reverse_cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
110+
$response = $this->client->get($server . '/test/reverse_cookie', $this->getHttpOpts())->getBody();
101111
$retrievedCookie = (int)$response;
102112

103113
if ($this->cookie === $retrievedCookie) {
@@ -117,7 +127,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
117127

118128
// test that the push server is a trusted proxy
119129
try {
120-
$resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
130+
$resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', $this->getHttpOpts())->getBody();
121131
} catch (\Exception $e) {
122132
$msg = $e->getMessage();
123133
$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -185,7 +195,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
185195
// test that the binary is up to date
186196
try {
187197
$this->queue->getConnection()->del('notify_push_version');
188-
$response = $this->client->post($server . '/test/version', ['nextcloud' => ['allow_local_address' => true], 'verify' => false]);
198+
$response = $this->client->post($server . '/test/version', $this->getHttpOpts());
189199
if ($response === 'error') {
190200
$output->writeln('<error>🗴 failed to get binary version, check the push server output for more information</error>');
191201
return self::ERROR_OTHER;

‎src/error.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
* SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
5-
5+
66
use flexi_logger::FlexiLoggerError;
77
use miette::Diagnostic;
88
use redis::RedisError;
99
use reqwest::StatusCode;
1010
use std::net::AddrParseError;
1111
use std::num::ParseIntError;
1212
use thiserror::Error;
13+
use warp::reject::Reject;
1314

1415
#[derive(Debug, Error, Diagnostic)]
1516
pub enum Error {
@@ -36,6 +37,8 @@ pub enum Error {
3637
SystemD(#[from] std::io::Error),
3738
}
3839

40+
impl Reject for Error {}
41+
3942
#[derive(Debug, Error, Diagnostic)]
4043
pub enum NextCloudError {
4144
#[error(transparent)]
@@ -68,6 +71,8 @@ pub enum DatabaseError {
6871

6972
#[derive(Debug, Error, Diagnostic)]
7073
pub enum SelfTestError {
74+
#[error("Invalid token")]
75+
Token,
7176
#[error("Failed to test database access: {0}")]
7277
Database(#[from] DatabaseError),
7378
#[error("Failed to test redis access: {0}")]

‎src/lib.rs‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
5-
65
use crate::config::{Bind, Config, TlsConfig};
76
use crate::connection::{handle_user_socket, ActiveConnections, ConnectionOptions};
87
pub use crate::error::Error;
@@ -36,7 +35,8 @@ use tokio::sync::{broadcast, oneshot};
3635
use tokio::time::sleep;
3736
use tokio_stream::wrappers::UnixListenerStream;
3837
use warp::filters::addr::remote;
39-
use warp::{Filter, Reply};
38+
use warp::reject::custom;
39+
use warp::{Filter, Rejection, Reply};
4040
use warp_real_ip::get_forwarded_for;
4141

4242
pub mod config;
@@ -268,6 +268,7 @@ pub fn serve(
268268

269269
let cookie_test = warp::path!("test" / "cookie")
270270
.and(app.clone())
271+
.and(test_token(app.clone()))
271272
.map(|app: Arc<App>| {
272273
let cookie = app.test_cookie.load(Ordering::SeqCst);
273274
log::debug!("current test cookie is {cookie}");
@@ -276,8 +277,10 @@ pub fn serve(
276277

277278
let reverse_cookie_test = warp::path!("test" / "reverse_cookie")
278279
.and(app.clone())
279-
.and_then(|app: Arc<App>| async move {
280-
let response = match app.nc_client.get_test_cookie().await {
280+
.and(test_token(app.clone()))
281+
.and(warp::header("token"))
282+
.and_then(|app: Arc<App>, token: String| async move {
283+
let response = match app.nc_client.get_test_cookie(&token).await {
281284
Ok(cookie) => {
282285
log::debug!("got remote test cookie {cookie}");
283286
cookie.to_string()
@@ -293,6 +296,7 @@ pub fn serve(
293296

294297
let mapping_test = warp::path!("test" / "mapping" / u32)
295298
.and(app.clone())
299+
.and(test_token(app.clone()))
296300
.and_then(|storage_id: u32, app: Arc<App>| async move {
297301
let access = app
298302
.storage_mapping
@@ -312,10 +316,12 @@ pub fn serve(
312316

313317
let remote_test = warp::path!("test" / "remote" / IpAddr)
314318
.and(app.clone())
315-
.and_then(|remote: IpAddr, app: Arc<App>| async move {
319+
.and(test_token(app.clone()))
320+
.and(warp::header("token"))
321+
.and_then(|remote: IpAddr, app: Arc<App>, token: String| async move {
316322
let result = app
317323
.nc_client
318-
.test_set_remote(remote)
324+
.test_set_remote(remote, &token)
319325
.await
320326
.map(|remote| remote.to_string())
321327
.unwrap_or_else(|e| e.to_string());
@@ -325,7 +331,8 @@ pub fn serve(
325331

326332
let version = warp::path!("test" / "version")
327333
.and(warp::post())
328-
.and(app)
334+
.and(app.clone())
335+
.and(test_token(app))
329336
.and_then(|app: Arc<App>| async move {
330337
Result::<_, Infallible>::Ok(match app.redis.connect().await {
331338
Ok(mut client) => {
@@ -451,3 +458,23 @@ pub async fn listen(app: Arc<App>) -> Result<()> {
451458
ping_handle.abort();
452459
Ok(())
453460
}
461+
462+
fn test_token(
463+
app: impl Filter<Extract = (Arc<App>,), Error = Infallible> + Clone,
464+
) -> impl Filter<Extract = (), Error = Rejection> + Clone {
465+
app.and(warp::header("token"))
466+
.and_then(|app: Arc<App>, token: String| async move {
467+
validate_token(&app, &token).await.map_err(custom)
468+
})
469+
.untuple_one()
470+
}
471+
472+
async fn validate_token(app: &App, token: &str) -> Result<()> {
473+
let mut redis = app.redis.connect().await?;
474+
let expected = redis.get("test-token").await?;
475+
if !token.is_empty() && token == expected {
476+
Ok(())
477+
} else {
478+
Err(SelfTestError::Token.into())
479+
}
480+
}

0 commit comments

Comments
 (0)