Release 1.3.1

appinfo/info.xml

@@ -11,7 +11,7 @@
     <description><![CDATA[Push update support for desktop app.
 
 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>
-    <version>1.3.0</version>
+    <version>1.3.1</version>
     <licence>agpl</licence>
     <author>Robin Appelman</author>
     <namespace>NotifyPush</namespace>

lib/Controller/TestController.php

@@ -10,11 +10,12 @@
 
 use OC\AppFramework\Http\Request;
 use OCA\NotifyPush\Queue\IQueue;
-use OCA\NotifyPush\Queue\RedisQueue;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http\Response;
 use OCP\IAppConfig;
 use OCP\IRequest;
 
@@ -33,10 +34,17 @@ public function __construct(
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 */
-	public function cookie(): DataResponse {
+	public function cookie(): Response {
 		// starting with 32, the app config does some internal caching
 		// that interferes with the quick set+get from this test.
 		$this->appConfig->clearCache();
+
+		$expected = $this->queue->get('test-token');
+		$token = $this->request->getHeader('token');
+		if ($expected !== $token) {
+			return new Response(Http::STATUS_FORBIDDEN);
+		}
+
 		return new DataResponse($this->appConfig->getValueInt('notify_push', 'cookie'));
 	}
 
@@ -45,12 +53,16 @@ public function cookie(): DataResponse {
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 */
-	public function remote(): DataDisplayResponse {
-		if ($this->queue instanceof RedisQueue) {
-			if ($this->request instanceof Request) {
-				$this->queue->getConnection()->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for'));
-				$this->queue->getConnection()->set('notify_push_remote', $this->request->server['REMOTE_ADDR']);
-			}
+	public function remote(): Response {
+		$expected = $this->queue->get('test-token');
+		$token = $this->request->getHeader('token');
+		if ($expected !== $token) {
+			return new Response(Http::STATUS_FORBIDDEN);
+		}
+
+		if ($this->request instanceof Request) {
+			$this->queue->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for'));
+			$this->queue->set('notify_push_remote', $this->request->server['REMOTE_ADDR']);
 		}
 		return new DataDisplayResponse($this->request->getRemoteAddress());
 	}
@@ -65,8 +77,6 @@ public function remote(): DataDisplayResponse {
 	 * @return void
 	 */
 	public function version(): void {
-		if ($this->queue instanceof RedisQueue) {
-			$this->queue->getConnection()->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push'));
-		}
+		$this->queue->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push'));
 	}
 }

lib/Queue/IQueue.php

@@ -10,9 +10,17 @@
 
 interface IQueue {
 	/**
-	 * @param string $channel
 	 * @param mixed $message
-	 * @return void
 	 */
-	public function push(string $channel, $message);
+	public function push(string $channel, $message): void;
+
+	/**
+	 * @param mixed $value
+	 */
+	public function set(string $key, $value): void;
+
+	/**
+	 * @return mixed
+	 */
+	public function get(string $key);
 }

lib/Queue/NullQueue.php

@@ -9,10 +9,19 @@
 namespace OCA\NotifyPush\Queue;
 
 class NullQueue implements IQueue {
-	/**
-	 * @return void
-	 */
-	public function push(string $channel, $message) {
+	#[\Override]
+	public function push(string $channel, $message): void {
 		// noop
 	}
+
+	#[\Override]
+	public function set(string $key, $value): void {
+		// noop
+	}
+
+	#[\Override]
+	public function get(string $key) {
+		return null;
+	}
+
 }

lib/Queue/RedisQueue.php

@@ -18,7 +18,8 @@ public function __construct($redis) {
 		$this->redis = $redis;
 	}
 
-	public function push(string $channel, $message) {
+	#[\Override]
+	public function push(string $channel, $message): void {
 		$this->redis->publish($channel, json_encode($message));
 	}
 
@@ -28,4 +29,14 @@ public function push(string $channel, $message) {
 	public function getConnection() {
 		return $this->redis;
 	}
+
+	#[\Override]
+	public function set(string $key, $value): void {
+		$this->redis->set($key, $value);
+	}
+
+	#[\Override]
+	public function get(string $key) {
+		return $this->redis->get($key);
+	}
 }

lib/SelfTest.php

@@ -12,19 +12,22 @@
 use OCA\NotifyPush\Queue\IQueue;
 use OCA\NotifyPush\Queue\RedisQueue;
 use OCP\App\IAppManager;
+use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\Security\ISecureRandom;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\HttpFoundation\IpUtils;
 
 class SelfTest {
 	public const ERROR_OTHER = 1;
 	public const ERROR_TRUSTED_PROXY = 2;
 
-	private $client;
-	private $cookie;
+	private IClient $client;
+	private int $cookie;
+	private string $token;
 
 	public function __construct(
 		IClientService $clientService,
@@ -33,9 +36,15 @@ public function __construct(
 		private IQueue $queue,
 		private IDBConnection $connection,
 		private IAppManager $appManager,
+		private ISecureRandom $random,
 	) {
 		$this->client = $clientService->newClient();
 		$this->cookie = rand(1, (int)pow(2, 30));
+		$this->token = $this->random->generate(32);
+	}
+
+	private function getHttpOpts(): array {
+		return ['nextcloud' => ['allow_local_address' => true], 'verify' => false, 'headers' => ['token' => $this->token]];
 	}
 
 	public function test(string $server, OutputInterface $output, bool $ignoreProxyError = false): int {
@@ -56,11 +65,12 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
 			$output->writeln('<comment>🗴 push server URL is set to localhost, the push server will not be reachable from other machines</comment>');
 		}
 
+		$this->queue->getConnection()->set('test-token', $this->token);
 		$this->queue->push('notify_test_cookie', $this->cookie);
 		$this->appConfig->setValueInt('notify_push', 'cookie', $this->cookie);
 
 		try {
-			$retrievedCookie = (int)$this->client->get($server . '/test/cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
+			$retrievedCookie = (int)$this->client->get($server . '/test/cookie', $this->getHttpOpts())->getBody();
 		} catch (\Exception $e) {
 			$msg = $e->getMessage();
 			$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -80,7 +90,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
 		// 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.
 		if ($storageId !== null) {
 			try {
-				$retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
+				$retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, $this->getHttpOpts())->getBody();
 			} catch (\Exception $e) {
 				$msg = $e->getMessage();
 				$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -97,7 +107,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
 
 		// test if the push server can reach nextcloud by having it request the cookie
 		try {
-			$response = $this->client->get($server . '/test/reverse_cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
+			$response = $this->client->get($server . '/test/reverse_cookie', $this->getHttpOpts())->getBody();
 			$retrievedCookie = (int)$response;
 
 			if ($this->cookie === $retrievedCookie) {
@@ -117,7 +127,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
 
 		// test that the push server is a trusted proxy
 		try {
-			$resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody();
+			$resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', $this->getHttpOpts())->getBody();
 		} catch (\Exception $e) {
 			$msg = $e->getMessage();
 			$output->writeln("<error>🗴 can't connect to push server: $msg</error>");
@@ -185,7 +195,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE
 		// test that the binary is up to date
 		try {
 			$this->queue->getConnection()->del('notify_push_version');
-			$response = $this->client->post($server . '/test/version', ['nextcloud' => ['allow_local_address' => true], 'verify' => false]);
+			$response = $this->client->post($server . '/test/version', $this->getHttpOpts());
 			if ($response === 'error') {
 				$output->writeln('<error>🗴 failed to get binary version, check the push server output for more information</error>');
 				return self::ERROR_OTHER;

src/error.rs

@@ -2,14 +2,15 @@
  * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
- 
+
 use flexi_logger::FlexiLoggerError;
 use miette::Diagnostic;
 use redis::RedisError;
 use reqwest::StatusCode;
 use std::net::AddrParseError;
 use std::num::ParseIntError;
 use thiserror::Error;
+use warp::reject::Reject;
 
 #[derive(Debug, Error, Diagnostic)]
 pub enum Error {
@@ -36,6 +37,8 @@ pub enum Error {
     SystemD(#[from] std::io::Error),
 }
 
+impl Reject for Error {}
+
 #[derive(Debug, Error, Diagnostic)]
 pub enum NextCloudError {
     #[error(transparent)]
@@ -68,6 +71,8 @@ pub enum DatabaseError {
 
 #[derive(Debug, Error, Diagnostic)]
 pub enum SelfTestError {
+    #[error("Invalid token")]
+    Token,
     #[error("Failed to test database access: {0}")]
     Database(#[from] DatabaseError),
     #[error("Failed to test redis access: {0}")]

src/lib.rs

@@ -2,7 +2,6 @@
  * SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-
 use crate::config::{Bind, Config, TlsConfig};
 use crate::connection::{handle_user_socket, ActiveConnections, ConnectionOptions};
 pub use crate::error::Error;
@@ -36,7 +35,8 @@ use tokio::sync::{broadcast, oneshot};
 use tokio::time::sleep;
 use tokio_stream::wrappers::UnixListenerStream;
 use warp::filters::addr::remote;
-use warp::{Filter, Reply};
+use warp::reject::custom;
+use warp::{Filter, Rejection, Reply};
 use warp_real_ip::get_forwarded_for;
 
 pub mod config;
@@ -268,6 +268,7 @@ pub fn serve(
 
     let cookie_test = warp::path!("test" / "cookie")
         .and(app.clone())
+        .and(test_token(app.clone()))
         .map(|app: Arc<App>| {
             let cookie = app.test_cookie.load(Ordering::SeqCst);
             log::debug!("current test cookie is {cookie}");
@@ -276,8 +277,10 @@ pub fn serve(
 
     let reverse_cookie_test = warp::path!("test" / "reverse_cookie")
         .and(app.clone())
-        .and_then(|app: Arc<App>| async move {
-            let response = match app.nc_client.get_test_cookie().await {
+        .and(test_token(app.clone()))
+        .and(warp::header("token"))
+        .and_then(|app: Arc<App>, token: String| async move {
+            let response = match app.nc_client.get_test_cookie(&token).await {
                 Ok(cookie) => {
                     log::debug!("got remote test cookie {cookie}");
                     cookie.to_string()
@@ -293,6 +296,7 @@ pub fn serve(
 
     let mapping_test = warp::path!("test" / "mapping" / u32)
         .and(app.clone())
+        .and(test_token(app.clone()))
         .and_then(|storage_id: u32, app: Arc<App>| async move {
             let access = app
                 .storage_mapping
@@ -312,10 +316,12 @@ pub fn serve(
 
     let remote_test = warp::path!("test" / "remote" / IpAddr)
         .and(app.clone())
-        .and_then(|remote: IpAddr, app: Arc<App>| async move {
+        .and(test_token(app.clone()))
+        .and(warp::header("token"))
+        .and_then(|remote: IpAddr, app: Arc<App>, token: String| async move {
             let result = app
                 .nc_client
-                .test_set_remote(remote)
+                .test_set_remote(remote, &token)
                 .await
                 .map(|remote| remote.to_string())
                 .unwrap_or_else(|e| e.to_string());
@@ -325,7 +331,8 @@ pub fn serve(
 
     let version = warp::path!("test" / "version")
         .and(warp::post())
-        .and(app)
+        .and(app.clone())
+        .and(test_token(app))
         .and_then(|app: Arc<App>| async move {
             Result::<_, Infallible>::Ok(match app.redis.connect().await {
                 Ok(mut client) => {
@@ -451,3 +458,23 @@ pub async fn listen(app: Arc<App>) -> Result<()> {
     ping_handle.abort();
     Ok(())
 }
+
+fn test_token(
+    app: impl Filter<Extract = (Arc<App>,), Error = Infallible> + Clone,
+) -> impl Filter<Extract = (), Error = Rejection> + Clone {
+    app.and(warp::header("token"))
+        .and_then(|app: Arc<App>, token: String| async move {
+            validate_token(&app, &token).await.map_err(custom)
+        })
+        .untuple_one()
+}
+
+async fn validate_token(app: &App, token: &str) -> Result<()> {
+    let mut redis = app.redis.connect().await?;
+    let expected = redis.get("test-token").await?;
+    if !token.is_empty() && token == expected {
+        Ok(())
+    } else {
+        Err(SelfTestError::Token.into())
+    }
+}