server: Add a test case for ensuring re-trying to unlock works

Author: bilelmoussaoui

server/src/collection.rs

@@ -1258,6 +1258,59 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn unlock_retry() -> Result<(), Box<dyn std::error::Error>> {
+        let setup = TestServiceSetup::plain_session(true).await?;
+        let default_collection = setup.default_collection().await?;
+
+        let secret = oo7::Secret::text("test-secret-data");
+        let dbus_secret = dbus::api::DBusSecret::new(Arc::clone(&setup.session), secret);
+        default_collection
+            .create_item("Test Item", &[("app", "test")], &dbus_secret, false, None)
+            .await?;
+
+        let collection = setup
+            .server
+            .collection_from_path(default_collection.inner().path())
+            .await
+            .expect("Collection should exist");
+        collection
+            .set_locked(true, setup.keyring_secret.clone())
+            .await?;
+
+        assert!(
+            default_collection.is_locked().await?,
+            "Collection should be locked"
+        );
+
+        setup
+            .mock_prompter
+            .set_password_queue(vec![
+                oo7::Secret::from("wrong-password"),
+                oo7::Secret::from("wrong-password2"),
+                oo7::Secret::from("test-password-long-enough"),
+            ])
+            .await;
+
+        let unlocked = setup
+            .service_api
+            .unlock(&[default_collection.inner().path()], None)
+            .await?;
+
+        assert_eq!(unlocked.len(), 1, "Should have unlocked 1 collection");
+        assert_eq!(
+            unlocked[0].as_str(),
+            default_collection.inner().path().as_str(),
+            "Should return the collection path"
+        );
+        assert!(
+            !default_collection.is_locked().await?,
+            "Collection should be unlocked after retry with correct password"
+        );
+
+        Ok(())
+    }
+
     #[tokio::test]
     async fn locked_collection_operations() -> Result<(), Box<dyn std::error::Error>> {
         let setup = TestServiceSetup::plain_session(true).await?;

server/src/gnome/prompter.rs

@@ -393,17 +393,15 @@ impl PrompterCallback {
     async fn prompter_done(&self, prompt: &Prompt, exchange: &str) -> Result<(), ServiceError> {
         let prompter = PrompterProxy::new(self.service.connection()).await?;
 
-        // Get the action from the prompt
-        let Some(action) = prompt.take_action().await else {
-            return Err(custom_service_error(
-                "Prompt action was already executed or not set",
-            ));
-        };
-
         // Handle each role differently based on what validation/preparation is needed
         match prompt.role() {
             PromptRole::Lock => {
-                // Lock operation doesn't need secret, just execute the action
+                let Some(action) = prompt.take_action().await else {
+                    return Err(custom_service_error(
+                        "Prompt action was already executed or not set",
+                    ));
+                };
+
                 let result_value = action.execute(None).await?;
 
                 let path = self.path.clone();
@@ -451,6 +449,13 @@ impl PrompterCallback {
 
                 if is_valid {
                     tracing::debug!("Keyring secret matches for {label}.");
+
+                    let Some(action) = prompt.take_action().await else {
+                        return Err(custom_service_error(
+                            "Prompt action was already executed or not set",
+                        ));
+                    };
+
                     // Execute the unlock action after successful validation
                     let result_value = action.execute(Some(secret)).await?;
 
@@ -507,6 +512,12 @@ impl PrompterCallback {
                     ));
                 };
 
+                let Some(action) = prompt.take_action().await else {
+                    return Err(custom_service_error(
+                        "Prompt action was already executed or not set",
+                    ));
+                };
+
                 // Execute the collection creation action with the secret
                 match action.execute(Some(secret)).await {
                     Ok(collection_path_value) => {

server/src/tests.rs

@@ -162,6 +162,8 @@ pub(crate) struct MockPrompterService {
     unlock_password: Arc<tokio::sync::Mutex<Option<oo7::Secret>>>,
     /// Whether to accept (true) or dismiss (false) prompts
     should_accept: Arc<tokio::sync::Mutex<bool>>,
+    /// Queue of passwords to use for for testing retry logic
+    password_queue: Arc<tokio::sync::Mutex<Vec<oo7::Secret>>>,
 }
 
 impl MockPrompterService {
@@ -171,13 +173,18 @@ impl MockPrompterService {
                 "test-password-long-enough",
             )))),
             should_accept: Arc::new(tokio::sync::Mutex::new(true)),
+            password_queue: Arc::new(tokio::sync::Mutex::new(Vec::new())),
         }
     }
 
     /// Set whether prompts should be accepted or dismissed
     pub async fn set_accept(&self, accept: bool) {
         *self.should_accept.lock().await = accept;
     }
+
+    pub async fn set_password_queue(&self, passwords: Vec<oo7::Secret>) {
+        *self.password_queue.lock().await = passwords;
+    }
 }
 
 #[zbus::interface(name = "org.gnome.keyring.internal.Prompter")]
@@ -240,6 +247,7 @@ impl MockPrompterService {
         let callback_path = callback.to_owned();
         let unlock_password = self.unlock_password.clone();
         let should_accept = self.should_accept.clone();
+        let password_queue = self.password_queue.clone();
         let exchange = exchange.to_owned();
         let connection = connection.clone();
 
@@ -266,24 +274,29 @@ impl MockPrompterService {
                     .await?;
                 tracing::debug!("MockPrompter: PromptReady(no) completed");
 
-                // Call PromptDone to clean up the callback
-                tracing::debug!("MockPrompter: calling PromptDone");
-                connection
-                    .call_method(
-                        None::<()>,
-                        &callback_path,
-                        Some("org.gnome.keyring.internal.Prompter.Callback"),
-                        "PromptDone",
-                        &(),
-                    )
-                    .await?;
-                tracing::debug!("MockPrompter: PromptDone completed");
-
                 return Ok(());
             } else if type_ == PromptType::Password {
                 tracing::debug!("MockPrompter: performing unlock (password prompt)");
                 // Unlock prompt - perform secret exchange
-                let password = unlock_password.lock().await.clone().unwrap();
+
+                let mut queue = password_queue.lock().await;
+                let password = if !queue.is_empty() {
+                    let pwd = queue.remove(0);
+                    tracing::debug!(
+                        "MockPrompter: using password from queue (length: {}, queue remaining: {})",
+                        std::str::from_utf8(pwd.as_bytes()).unwrap_or("<binary>"),
+                        queue.len()
+                    );
+                    pwd
+                } else {
+                    let pwd = unlock_password.lock().await.clone().unwrap();
+                    tracing::debug!(
+                        "MockPrompter: using default password (length: {})",
+                        std::str::from_utf8(pwd.as_bytes()).unwrap_or("<binary>")
+                    );
+                    pwd
+                };
+                drop(queue);
 
                 // Generate our own key pair
                 let private_key = oo7::Key::generate_private_key().unwrap();
@@ -330,27 +343,40 @@ impl MockPrompterService {
                 tracing::debug!("MockPrompter: PromptReady(yes) completed");
             }
 
-            // Call PromptDone to clean up the callback
-            tracing::debug!("MockPrompter: calling PromptDone");
-            connection
+            Ok::<_, zbus::Error>(())
+        });
+
+        Ok(())
+    }
+
+    async fn stop_prompting(
+        &self,
+        callback: ObjectPath<'_>,
+        #[zbus(connection)] connection: &zbus::Connection,
+    ) -> zbus::fdo::Result<()> {
+        tracing::debug!("MockPrompter: stop_prompting called for {}", callback);
+        let callback_path = callback.to_owned();
+        let connection = connection.clone();
+
+        tokio::spawn(async move {
+            tracing::debug!("MockPrompter: calling PromptDone for {}", callback_path);
+            let result = connection
                 .call_method(
                     None::<()>,
                     &callback_path,
                     Some("org.gnome.keyring.internal.Prompter.Callback"),
                     "PromptDone",
                     &(),
                 )
-                .await?;
-            tracing::debug!("MockPrompter: PromptDone completed");
+                .await;
 
-            Ok::<_, zbus::Error>(())
+            if let Err(err) = result {
+                tracing::debug!("MockPrompter: PromptDone failed: {}", err);
+            } else {
+                tracing::debug!("MockPrompter: PromptDone completed for {}", callback_path);
+            }
         });
 
         Ok(())
     }
-
-    async fn stop_prompting(&self, _callback: ObjectPath<'_>) -> zbus::fdo::Result<()> {
-        // Called when prompting is complete
-        Ok(())
-    }
 }