Merge pull request #282 from adorsys/277-prevent-accidental-exposure-…

…of-internal-details-in-the-oob-messages-plugin

Prevent accidental exposure of internal details in the oob messages plugin

crates/filesystem/src/lib.rs

@@ -8,7 +8,7 @@ use std::{
 
 #[doc(hidden)]
 // Define a trait for file system operations
-pub trait FileSystem: Send + 'static {
+pub trait FileSystem: Send + Sync + 'static {
     fn read_to_string(&self, path: &Path) -> IoResult<String>;
     fn write(&mut self, path: &Path, content: &str) -> IoResult<()>;
     fn read_dir_files(&self, path: &Path) -> IoResult<Vec<String>>;

crates/web-plugins/oob-messages/Cargo.toml

@@ -20,6 +20,7 @@ serde = { workspace = true, features = ["derive"] }
 uuid = { workspace = true, features = ["fast-rng", "v4"] }
 
 [dev-dependencies]
+filesystem = { workspace = true, features = ["test-utils"] }
 mockall = "0.13.0"
 tokio = { version = "1.30.0", features = ["full"] }
 tower = "0.4"

crates/web-plugins/oob-messages/src/models.rs

@@ -8,7 +8,8 @@ use multibase::Base::Base64Url;
 use qrcode::QrCode;
 use serde::{Deserialize, Serialize};
 use serde_json::to_string;
-use std::{collections::HashMap, sync::Mutex};
+use std::collections::HashMap;
+use std::sync::RwLock;
 
 #[cfg(test)]
 use std::io::Result as IoResult;
@@ -20,8 +21,8 @@ use std::io::Result as IoResult;
 // The out-of-band protocol consists in a single message that is sent by the sender.
 
 // This is the first step in the interaction with the Mediator. The following one is the mediation coordination where a 'request mediation' request is created and performed.
-/// e.g.:
-/// ```
+// e.g.:
+// ```
 // {
 //     "type": "https://didcomm.org/out-of-band/2.0/invitation",
 //     "id": "0a2c57a5-5662-48a8-bca8-78275cef3c80",
@@ -35,7 +36,7 @@ use std::io::Result as IoResult;
 //       ]
 //     }
 //   }
-/// ```
+// ```
 
 #[derive(Debug, Serialize, Deserialize, Clone)]
 struct OobMessage {
@@ -83,12 +84,15 @@ impl OobMessage {
 }
 
 // Receives server path/port and local storage path and returns a String with the OOB URL.
-pub(crate) fn retrieve_or_generate_oob_inv<'a>(
-    fs: &mut dyn FileSystem,
+pub(crate) fn retrieve_or_generate_oob_inv<F>(
+    fs: &mut F,
     server_public_domain: &str,
     server_local_port: &str,
     storage_dirpath: &str,
-) -> Result<String, String> {
+) -> Result<String, String>
+where
+    F: FileSystem + ?Sized,
+{
     // Construct the file path
     let file_path = format!("{}/oob_invitation.txt", storage_dirpath);
 
@@ -104,35 +108,38 @@ pub(crate) fn retrieve_or_generate_oob_inv<'a>(
     let diddoc: Document = fs
         .read_to_string(diddoc_path.as_ref())
         .map(|content| serde_json::from_str(&content).unwrap())
-        .map_err(|e| format!("Failed to read DID document: {}", e))?;
+        .map_err(|err| format!("Failed to read DID document: {err}"))?;
 
     let did = diddoc.id.clone();
     let oob_message = OobMessage::new(&did);
     let url: &String = &format!("{}:{}", server_public_domain, server_local_port);
     let oob_url = OobMessage::serialize_oob_message(&oob_message, url)
-        .map_err(|e| format!("Serialization error: {}", e))?;
+        .map_err(|err| format!("Serialization error: {err}"))?;
 
     // Attempt to create the file and write the string
-    to_local_storage(fs, &oob_url, storage_dirpath);
+    to_local_storage(fs, &oob_url, storage_dirpath)?;
 
     Ok(oob_url)
 }
 
 lazy_static! {
-    static ref CACHE: Mutex<HashMap<String, String>> = Mutex::new(HashMap::new());
+    static ref CACHE: RwLock<HashMap<String, String>> = RwLock::new(HashMap::new());
 }
 
 // Function to generate and save a QR code image with caching
-pub(crate) fn retrieve_or_generate_qr_image(
-    fs: &mut dyn FileSystem,
+pub(crate) fn retrieve_or_generate_qr_image<F>(
+    fs: &mut F,
     base_path: &str,
     url: &str,
-) -> Result<String, String> {
+) -> Result<String, String>
+where
+    F: FileSystem + ?Sized,
+{
     let path = format!("{}/qrcode.txt", base_path);
 
     // Check the cache first
     {
-        let cache = CACHE.lock().map_err(|e| format!("Cache error: {}", e))?;
+        let cache = CACHE.read().map_err(|err| format!("Cache error: {err}"))?;
         if let Some(existing_image) = cache.get(&path) {
             return Ok(existing_image.clone());
         }
@@ -142,15 +149,15 @@ pub(crate) fn retrieve_or_generate_qr_image(
     if let Ok(existing_image) = fs.read_to_string(path.as_ref()) {
         // Update the cache with the retrieved data
         CACHE
-            .lock()
-            .map_err(|e| format!("Cache error: {:?}", e))?
+            .write()
+            .map_err(|err| format!("Cache error: {err}"))?
             .insert(path.clone(), existing_image.clone());
         return Ok(existing_image);
     }
 
     // Generate QR code
     let qr_code = QrCode::new(url.as_bytes())
-        .map_err(|error| format!("Failed to generate QR code: {:?}", error))?;
+        .map_err(|error| format!("Failed to generate QR code: {error:?}"))?;
 
     let image = qr_code.render::<Luma<u8>>().build();
 
@@ -159,37 +166,38 @@ pub(crate) fn retrieve_or_generate_qr_image(
     let mut buffer = Vec::new();
     dynamic_image
         .write_to(&mut buffer, image::ImageOutputFormat::Png)
-        .expect("Error encoding image to PNG");
+        .map_err(|err| format!("Error encoding image to PNG: {err}"))?;
 
     // Save the PNG-encoded byte vector as a base64-encoded string
     let base64_string = encode_config(&buffer, STANDARD);
 
     // Save to file
     fs.write_with_lock(path.as_ref(), &base64_string)
-        .map_err(|e| format!("Error writing: {:?}", e))?;
+        .map_err(|err| format!("Error writing: {err:?}"))?;
     CACHE
-        .lock()
-        .map_err(|e| format!("Cache error: {:?}", e))?
+        .write()
+        .map_err(|err| format!("Cache error: {err:?}"))?
         .insert(path.clone(), base64_string.clone());
 
     Ok(base64_string)
 }
 
-fn to_local_storage(fs: &mut dyn FileSystem, oob_url: &str, storage_dirpath: &str) {
+fn to_local_storage<F>(fs: &mut F, oob_url: &str, storage_dirpath: &str) -> Result<(), String>
+where
+    F: FileSystem + ?Sized,
+{
     // Ensure the parent directory ('storage') exists
-    if let Err(e) = fs.create_dir_all(storage_dirpath.as_ref()) {
-        tracing::error!("Error creating directory: {:?}", e);
-        return;
-    }
+    fs.create_dir_all(storage_dirpath.as_ref())
+        .map_err(|err| format!("Error creating directory: {err}"))?;
 
     let file_path = format!("{}/oob_invitation.txt", storage_dirpath);
 
     // Attempt to write the string directly to the file
-    if let Err(e) = fs.write(file_path.as_ref(), oob_url) {
-        tracing::error!("Error writing to file: {:?}", e);
-    } else {
-        tracing::info!("String successfully written to file.");
-    }
+    fs.write(file_path.as_ref(), oob_url)
+        .map_err(|err| format!("Error writing to file: {err}"))?;
+    tracing::info!("String successfully written to file.");
+
+    Ok(())
 }
 
 #[cfg(test)]

crates/web-plugins/oob-messages/src/plugin.rs

@@ -1,52 +1,82 @@
+use std::sync::{Arc, Mutex};
+
 use super::{
     models::{retrieve_or_generate_oob_inv, retrieve_or_generate_qr_image},
     web,
 };
 use axum::Router;
-use filesystem::StdFileSystem;
+use filesystem::{FileSystem, StdFileSystem};
 use plugin_api::{Plugin, PluginError};
 
 #[derive(Default)]
-pub struct OOBMessages;
+pub struct OOBMessages {
+    env: Option<OOBMessagesEnv>,
+    state: Option<OOBMessagesState>,
+}
+
+struct OOBMessagesEnv {
+    storage_dirpath: String,
+    server_public_domain: String,
+    server_local_port: String,
+}
+
+#[derive(Clone)]
+pub(crate) struct OOBMessagesState {
+    pub(crate) filesystem: Arc<Mutex<dyn FileSystem>>,
+}
+
+fn get_env() -> Result<OOBMessagesEnv, PluginError> {
+    let storage_dirpath = std::env::var("STORAGE_DIRPATH")
+        .map_err(|_| PluginError::InitError("STORAGE_DIRPATH env variable required".to_owned()))?;
+
+    let server_public_domain = std::env::var("SERVER_PUBLIC_DOMAIN").map_err(|_| {
+        PluginError::InitError("SERVER_PUBLIC_DOMAIN env variable required".to_owned())
+    })?;
+
+    let server_local_port = std::env::var("SERVER_LOCAL_PORT").map_err(|_| {
+        PluginError::InitError("SERVER_LOCAL_PORT env variable required".to_owned())
+    })?;
+
+    Ok(OOBMessagesEnv {
+        storage_dirpath,
+        server_public_domain,
+        server_local_port,
+    })
+}
 
 impl Plugin for OOBMessages {
     fn name(&self) -> &'static str {
         "oob_messages"
     }
 
     fn mount(&mut self) -> Result<(), PluginError> {
+        let env = get_env()?;
         let mut fs = StdFileSystem;
 
-        let server_public_domain = std::env::var("SERVER_PUBLIC_DOMAIN").map_err(|_| {
-            PluginError::InitError("SERVER_PUBLIC_DOMAIN env variable required".to_owned())
-        })?;
-
-        let server_local_port = std::env::var("SERVER_LOCAL_PORT").map_err(|_| {
-            PluginError::InitError("SERVER_LOCAL_PORT env variable required".to_owned())
-        })?;
-
-        let storage_dirpath = std::env::var("STORAGE_DIRPATH").map_err(|_| {
-            PluginError::InitError("STORAGE_DIRPATH env variable required".to_owned())
-        })?;
-
         let oob_inv = retrieve_or_generate_oob_inv(
             &mut fs,
-            &server_public_domain,
-            &server_local_port,
-            &storage_dirpath,
+            &env.server_public_domain,
+            &env.server_local_port,
+            &env.storage_dirpath,
         )
-        .map_err(|e| {
+        .map_err(|err| {
             PluginError::InitError(format!(
-                "Error retrieving or generating OOB invitation: {e}"
+                "Error retrieving or generating OOB invitation: {err}"
             ))
         })?;
 
         tracing::debug!("Out Of Band Invitation: {}", oob_inv);
 
-        let _ =
-            retrieve_or_generate_qr_image(&mut fs, &storage_dirpath, &oob_inv).map_err(|e| {
-                PluginError::InitError(format!("Error retrieving or generating QR code image: {e}"))
-            })?;
+        retrieve_or_generate_qr_image(&mut fs, &env.storage_dirpath, &oob_inv).map_err(|err| {
+            PluginError::InitError(format!(
+                "Error retrieving or generating QR code image: {err}"
+            ))
+        })?;
+
+        self.env = Some(env);
+        self.state = Some(OOBMessagesState {
+            filesystem: Arc::new(Mutex::new(fs)),
+        });
 
         Ok(())
     }
@@ -56,6 +86,9 @@ impl Plugin for OOBMessages {
     }
 
     fn routes(&self) -> Result<Router, PluginError> {
-        Ok(web::routes())
+        let state = self.state.as_ref().ok_or(PluginError::Other(
+            "missing state, plugin not mounted".to_owned(),
+        ))?;
+        Ok(web::routes(Arc::new(state.clone())))
     }
 }

crates/web-plugins/oob-messages/src/web.rs

@@ -1,28 +1,41 @@
 pub(crate) mod handler;
-
-use crate::web::handler::{handler_landing_page_oob, handler_oob_inv, handler_oob_qr};
+use crate::{
+    plugin::OOBMessagesState,
+    web::handler::{handler_landing_page_oob, handler_oob_inv, handler_oob_qr},
+};
 use axum::{routing::get, Router};
+use std::sync::Arc;
 
-pub(crate) fn routes() -> Router {
+pub(crate) fn routes(state: Arc<OOBMessagesState>) -> Router {
     Router::new() //
         .route("/oob_url", get(handler_oob_inv))
         .route("/oob_qr", get(handler_oob_qr))
         .route("/", get(handler_landing_page_oob))
+        .with_state(state)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-
     use axum::{
         body::Body,
         http::{Request, StatusCode},
     };
+    use filesystem::MockFileSystem;
+    use std::sync::Mutex;
     use tower::util::ServiceExt;
 
     #[tokio::test]
     async fn test_routes() {
-        let app = routes();
+        std::env::set_var("STORAGE_DIRPATH", "tmp");
+        std::env::set_var("SERVER_PUBLIC_DOMAIN", "example.com");
+        std::env::set_var("SERVER_LOCAL_PORT", "8080");
+
+        let fs = MockFileSystem;
+        let state = Arc::new(OOBMessagesState {
+            filesystem: Arc::new(Mutex::new(fs)),
+        });
+        let app = routes(state.clone());
 
         let response = app
             .oneshot(
@@ -36,7 +49,7 @@ mod tests {
 
         assert_eq!(response.status(), StatusCode::OK);
 
-        let app = routes();
+        let app = routes(state.clone());
 
         let response = app
             .oneshot(
@@ -50,7 +63,7 @@ mod tests {
 
         assert_eq!(response.status(), StatusCode::OK);
 
-        let app = routes();
+        let app = routes(state);
 
         let response = app
             .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap())