refactor(http)!: always enforce the scope on redirects (#3600)

* refactor(http)!: always enforce the scope on redirects

Removes the `scopeRedirects` option (and the `Config` struct with it) that
was added as an opt-in in 2.7.0. Every hop of a redirect chain is now
checked against the URL scope, so a server on an allowed origin can no
longer redirect the request to a URL the scope denies.

`tauri_plugin_http::init()` returns `TauriPlugin<R>` again.

* chore(http): compile without warnings when the cookies feature is disabled

.changes/http-scope-redirects-default.md

@@ -0,0 +1,6 @@
+---
+"http": major
+"http-js": major
+---
+
+**Breaking:** The URL scope is now always checked on every hop of a redirect chain, so every redirect target must be allowed by the scope or the request fails with `url not allowed on the configured scope`. This was previously opt-in through the `scopeRedirects` plugin configuration, which has been removed along with the `Config` struct: `tauri_plugin_http::init()` returns `TauriPlugin<R>` again, and any `plugins > http` object must be removed from `tauri.conf.json`.

plugins/http/guest-js/index.ts

@@ -77,9 +77,8 @@ export interface ClientOptions {
    * Defines the maximum number of redirects the client should follow.
    * If set to 0, no redirects will be followed.
    *
-   * When the `scopeRedirects` plugin configuration is enabled, every redirect must
+   * Every redirect must also be allowed by the configured scope,
-   * also be allowed by the configured scope, otherwise the request fails
+   * otherwise the request fails instead of being followed.
-   * instead of being followed.
    */
   maxRedirections?: number
   /** Timeout in milliseconds */

plugins/http/src/commands.rs

@@ -180,25 +180,17 @@ fn attach_proxy(
 
 /// Builds the redirect policy for the request.
 ///
-/// When `scope` is [`Some`], **every** hop is checked against it. Validating only the URL the
+/// **Every** hop is checked against the scope. Validating only the URL the frontend asked for is
-/// frontend asked for is not enough: a server on an allowed origin can answer with a redirect to
+/// not enough: a server on an allowed origin can answer with a redirect to any other origin (an
-/// any other origin (an open redirect, or a server the attacker controls), and following it would
+/// open redirect, or a server the attacker controls), and following it would give the webview
-/// give the webview access to a URL the scope denies.
+/// access to a URL the scope denies.
-///
+fn redirect_policy(scope: Scope, max_redirections: Option<usize>) -> Policy {
-/// That check is opt-in through the `scopeRedirects` configuration because it breaks applications
-/// that rely on being redirected outside of their scope.
-// TODO(v3): always check the scope and take it by value instead of `Option`
-fn redirect_policy(scope: Option<Scope>, max_redirections: Option<usize>) -> Policy {
     if max_redirections == Some(0) {
         return Policy::none();
     }
 
     let max_redirections = max_redirections.unwrap_or(DEFAULT_MAX_REDIRECTIONS);
 
-    let Some(scope) = scope else {
-        return Policy::limited(max_redirections);
-    };
-
     Policy::custom(move |attempt| {
         // the first URL in `previous` is the initial request, so it must be excluded
         if attempt.previous().len() > max_redirections {
@@ -228,6 +220,8 @@ fn map_request_error(error: reqwest::Error) -> Error {
     Error::Network(error)
 }
 
+// `state` is only read when the `cookies` feature is enabled
+#[cfg_attr(not(feature = "cookies"), allow(unused_variables))]
 #[command]
 pub async fn fetch<R: Runtime>(
     webview: Webview<R>,
@@ -311,7 +305,6 @@ pub async fn fetch<R: Runtime>(
                 builder = builder.connect_timeout(Duration::from_millis(timeout));
             }
 
-            let scope = state.config.scope_redirects.then_some(scope);
             builder = builder.redirect(redirect_policy(scope, max_redirections));
 
             if let Some(proxy_config) = proxy {
@@ -605,11 +598,7 @@ mod tests {
         port
     }
 
-    fn get(
+    fn get(url: &str, scope: Scope, max_redirections: Option<usize>) -> Result<reqwest::Response> {
-        url: &str,
-        scope: Option<Scope>,
-        max_redirections: Option<usize>,
-    ) -> Result<reqwest::Response> {
         let client = reqwest::ClientBuilder::new()
             .redirect(redirect_policy(scope, max_redirections))
             .build()
@@ -620,9 +609,9 @@ mod tests {
         )
     }
 
-    fn localhost_scope(port: u16) -> Option<Scope> {
+    fn localhost_scope(port: u16) -> Scope {
         let entry = Arc::new(format!("http://localhost:{port}/*").parse().unwrap());
-        Some(Scope::new(vec![entry], Vec::new()))
+        Scope::new(vec![entry], Vec::new())
     }
 
     #[test]
@@ -644,27 +633,12 @@ mod tests {
         }
     }
 
-    #[test]
-    fn redirect_outside_of_scope_is_followed_when_not_configured() {
-        let port = spawn_server();
-
-        // `scopeRedirects` is disabled, so only the URL requested by the frontend is checked
-        let response = get(
-            &format!("http://localhost:{port}/redirect-external"),
-            None,
-            None,
-        )
-        .unwrap();
-
-        assert_eq!(response.status(), StatusCode::OK);
-    }
-
     #[test]
     fn redirect_denied_by_the_scope_is_denied() {
         let port = spawn_server();
         let allow = Arc::new(format!("http://localhost:{port}/*").parse().unwrap());
         let deny = Arc::new(format!("http://localhost:{port}/target").parse().unwrap());
-        let scope = Some(Scope::new(vec![allow], vec![deny]));
+        let scope = Scope::new(vec![allow], vec![deny]);
 
         let err = get(&format!("http://localhost:{port}/redirect/0"), scope, None).unwrap_err();
 
@@ -690,16 +664,14 @@ mod tests {
     fn max_redirections_is_enforced() {
         let port = spawn_server();
 
-        for scope in [localhost_scope(port), None] {
+        let err = get(
-            let err = get(
+            &format!("http://localhost:{port}/redirect/5"),
-                &format!("http://localhost:{port}/redirect/5"),
+            localhost_scope(port),
-                scope,
+            Some(2),
-                Some(2),
+        )
-            )
+        .unwrap_err();
-            .unwrap_err();
 
-            assert!(matches!(err, Error::Network(e) if e.is_redirect()));
+        assert!(matches!(err, Error::Network(e) if e.is_redirect()));
-        }
     }
 
     #[test]

plugins/http/src/config.rs

@@ -1,47 +0,0 @@
-// Copyright 2019-2023 Tauri Programme within The Commons Conservancy
-// SPDX-License-Identifier: Apache-2.0
-// SPDX-License-Identifier: MIT
-
-use serde::Deserialize;
-
-/// HTTP plugin configuration, defined on the `plugins > http` object of your `tauri.conf.json`.
-#[derive(Debug, Clone, Default, Deserialize)]
-#[serde(rename_all = "camelCase", deny_unknown_fields)]
-pub struct Config {
-    /// Whether the scope is checked on every hop of a redirect chain instead of only on the
-    /// URL requested by the frontend. Defaults to `false`.
-    ///
-    /// When disabled, a server on an allowed origin can answer with a redirect to any other
-    /// origin - an open redirect, or a server an attacker controls - and the plugin follows it,
-    /// handing the webview a response from a URL the scope denies, such as a `localhost`
-    /// service, an internal host or a cloud metadata endpoint.
-    ///
-    /// When enabled, a redirect to a URL that is not allowed by the scope fails with
-    /// [`Error::UrlNotAllowed`](crate::Error::UrlNotAllowed) instead of being followed, so every
-    /// redirect target must also be part of the scope. This is opt-in because it breaks
-    /// applications that rely on being redirected outside of their configured scope.
-    // TODO(v3): enforce the scope on redirects by default and remove this option
-    #[serde(default)]
-    pub scope_redirects: bool,
-}
-
-#[cfg(test)]
-mod tests {
-    use super::Config;
-
-    #[test]
-    fn deserializes_the_plugin_configuration() {
-        // the plugin configuration is optional, and the scope check is opt-in
-        let config: Option<Config> = serde_json::from_value(serde_json::Value::Null).unwrap();
-        assert!(config.is_none());
-
-        let config: Config = serde_json::from_str("{}").unwrap();
-        assert!(!config.scope_redirects);
-
-        let config: Config = serde_json::from_str(r#"{ "scopeRedirects": true }"#).unwrap();
-        assert!(config.scope_redirects);
-
-        // a typo must not silently disable the scope check
-        assert!(serde_json::from_str::<Config>(r#"{ "scopeRedirect": true }"#).is_err());
-    }
-}

plugins/http/src/lib.rs

@@ -44,20 +44,12 @@
 //! - **unsafe-headers**: Allows webview requests to send any headers.
 //! - **dangerous-settings**: Allows dangerous client settings such as accepting invalid certificates or hostnames.
 //!
-//! ## Configuration
+//! ## Security
 //!
-//! See [`Config`] for the options that can be set on the `plugins > http` object of your
+//! The URL scope is checked on every hop of a redirect chain, not only on the URL requested by
-//! `tauri.conf.json`:
+//! the frontend, so every redirect target must also be allowed by the scope. Otherwise a server on
-//!
+//! an allowed origin could redirect the request to any other origin - a `localhost` service, an
-//! ```json
+//! internal host or a cloud metadata endpoint - and hand its response to the webview.
-//! {
-//!   "plugins": {
-//!     "http": {
-//!       "scopeRedirects": true
-//!     }
-//!   }
-//! }
-//! ```
 
 pub use reqwest;
 use tauri::{
@@ -65,11 +57,9 @@ use tauri::{
     Manager, Runtime,
 };
 
-pub use config::Config;
 pub use error::{Error, Result};
 
 mod commands;
-mod config;
 mod error;
 #[cfg(feature = "cookies")]
 mod reqwest_cookie_store;
@@ -79,14 +69,13 @@ mod scope;
 const COOKIES_FILENAME: &str = ".cookies";
 
 pub(crate) struct Http {
-    pub(crate) config: Config,
     #[cfg(feature = "cookies")]
     cookies_jar: std::sync::Arc<crate::reqwest_cookie_store::CookieStoreMutex>,
 }
 
-pub fn init<R: Runtime>() -> TauriPlugin<R, Option<Config>> {
+pub fn init<R: Runtime>() -> TauriPlugin<R> {
-    Builder::<R, Option<Config>>::new("http")
+    Builder::new("http")
-        .setup(|app, api| {
+        .setup(|app, _api| {
             #[cfg(feature = "cookies")]
             let cookies_jar = {
                 use crate::reqwest_cookie_store::*;
@@ -114,7 +103,6 @@ pub fn init<R: Runtime>() -> TauriPlugin<R, Option<Config>> {
             };
 
             let state = Http {
-                config: api.config().clone().unwrap_or_default(),
                 #[cfg(feature = "cookies")]
                 cookies_jar: std::sync::Arc::new(cookies_jar),
             };
@@ -138,6 +126,9 @@ pub fn init<R: Runtime>() -> TauriPlugin<R, Option<Config>> {
                     }
                 }
             }
+
+            #[cfg(not(feature = "cookies"))]
+            let _ = (app, event);
         })
         .invoke_handler(tauri::generate_handler![
             commands::fetch,