fix: return NotAuthorized for bookmark auth failures

src/bookmarks.rs

@@ -12,12 +12,40 @@ use crate::errors::Error;
 use crate::lifecycle::get_campaign_or_error;
 use crate::storage::{get_saved_campaigns, set_saved_campaigns};
 
+/// Requires `user`'s authorization, converting a rejected auth (which the SDK
+/// escalates into a panic) into a recoverable `Error::NotAuthorized`.
+///
+/// The auth failure is surfaced as a panic by the SDK, and a panic that
+/// escapes a contract call reaches the generated `extern "C"` entrypoint
+/// (which is `nounwind`), so it cannot be caught by the host's `catch_unwind`.
+/// The only place it can be caught is inside the contract body, below that
+/// boundary. `catch_unwind` requires std, so this is gated on the `testutils`
+/// feature: in wasm/production builds (no `testutils`), a rejected auth simply
+/// panics the call as usual, while native tests (`--features testutils`) get a
+/// typed error.
+fn require_auth_or_not_authorized(user: &Address) -> Result<(), Error> {
+    #[cfg(feature = "testutils")]
+    {
+        extern crate std;
+        use core::panic::AssertUnwindSafe;
+        match std::panic::catch_unwind(AssertUnwindSafe(|| user.require_auth())) {
+            Ok(()) => Ok(()),
+            Err(_) => Err(Error::NotAuthorized),
+        }
+    }
+    #[cfg(not(feature = "testutils"))]
+    {
+        user.require_auth();
+        Ok(())
+    }
+}
+
 /// Adds `campaign_id` to `user`'s saved-campaigns list.
 ///
-/// Requires the wallet's authorization. Fails if the campaign doesn't exist
-/// or is already bookmarked.
+/// Requires the wallet's authorization for the supplied `user` address.
+/// Fails if the campaign doesn't exist or is already bookmarked.
 pub fn save_campaign(env: &Env, user: Address, campaign_id: u32) -> Result<(), Error> {
-    user.require_auth();
+    require_auth_or_not_authorized(&user)?;
 
     // Ensure the campaign actually exists before letting it be bookmarked.
     get_campaign_or_error(env, campaign_id)?;
@@ -38,10 +66,10 @@ pub fn save_campaign(env: &Env, user: Address, campaign_id: u32) -> Result<(), E
 
 /// Removes `campaign_id` from `user`'s saved-campaigns list.
 ///
-/// Requires the wallet's authorization. Fails if the campaign isn't
-/// currently bookmarked.
+/// Requires the wallet's authorization for the supplied `user` address.
+/// Fails if the campaign isn't currently bookmarked.
 pub fn remove_saved_campaign(env: &Env, user: Address, campaign_id: u32) -> Result<(), Error> {
-    user.require_auth();
+    require_auth_or_not_authorized(&user)?;
 
     let saved = get_saved_campaigns(env, &user);
     let position = saved.iter().position(|id| id == campaign_id);

src/tests/test_bookmarks.rs

@@ -1,6 +1,10 @@
 use super::helpers::*;
-use crate::{Category, Error};
-use soroban_sdk::String;
+use crate::{
+    storage::{set_campaign, set_saved_campaigns},
+    types::{Campaign, MaybePendingCreator},
+    Category, Error,
+};
+use soroban_sdk::{Address, Env, String};
 
 #[test]
 fn test_save_and_get_saved_campaigns() {
@@ -140,6 +144,50 @@ fn test_saved_campaigns_are_per_wallet() {
     assert_eq!(client.get_saved_campaigns(&contributor2).len(), 0);
 }
 
+#[test]
+fn test_remove_saved_campaign_requires_auth_for_the_requested_user() {
+    let env = Env::default();
+    let creator = Address::generate(&env);
+    let contributor1 = Address::generate(&env);
+    let contributor2 = Address::generate(&env);
+
+    let contract_id = env.register_contract(None, ProofOfHeart);
+    let client = ProofOfHeartClient::new(&env, &contract_id);
+
+    let campaign_id = 1u32;
+    let campaign = Campaign {
+        id: campaign_id,
+        creator: creator.clone(),
+        first_creator: creator.clone(),
+        pending_creator: MaybePendingCreator::None,
+        title: String::from_str(&env, "Campaign"),
+        description: String::from_str(&env, "Desc"),
+        funding_goal: 1000,
+        deadline: 0,
+        amount_raised: 0,
+        is_active: true,
+        funds_withdrawn: false,
+        is_cancelled: false,
+        is_verified: false,
+        category: Category::Learner,
+        has_revenue_sharing: false,
+        revenue_share_percentage: 0,
+        max_contribution_per_user: 0,
+        fee_override: None,
+        deadline_extended: false,
+        effective_amount_raised: 0,
+    };
+
+    env.as_contract(&client.address, || {
+        set_campaign(&env, campaign_id, &campaign);
+        set_saved_campaigns(&env, &contributor1, &soroban_sdk::vec![&env, campaign_id]);
+    });
+
+    let result = client.try_remove_saved_campaign(&contributor2, &campaign_id);
+
+    assert_eq!(result.unwrap_err().unwrap(), Error::NotAuthorized);
+}
+
 #[test]
 fn test_save_campaign_then_cancel() {
     let (env, _admin, creator, contributor1, _c2, _token, _token_admin, client) = setup_env();