fix(user_registry): guard DID wallet rotation against reverse-index c…

contracts/contracts/user_registry/src/lib.rs

@@ -443,7 +443,23 @@ impl UserRegistryContract {
     }
 
     /// Update the wallet address for an existing Privy DID mapping.
-    /// Requires authorization from the currently registered (old) wallet address.
+    ///
+    /// Issue #982: lets a user rotate the wallet linked to their Privy DID
+    /// (e.g. after a key rotation inside Privy) without going through admin
+    /// recovery. Authorization is enforced by requiring the *currently
+    /// registered* old wallet to sign the call (`old_wallet.require_auth()`)
+    /// and confirming it actually matches the stored forward mapping — a
+    /// caller cannot move someone else's DID by simply naming their own
+    /// address as `old_wallet`.
+    ///
+    /// Also rejects the move if `new_wallet` already has a *different* DID
+    /// linked (via the `WalletDid` reverse index). Without this check, the
+    /// call would silently overwrite that other DID's reverse mapping —
+    /// leaving its forward mapping (`PrivyDid(other_did) -> new_wallet`)
+    /// pointing at a wallet whose reverse index no longer agrees, an
+    /// inconsistency `register_privy_did` already guards against on first
+    /// link (see its own duplicate-wallet check) but this rotation path
+    /// previously did not.
     pub fn update_privy_did(env: Env, did: String, old_wallet: Address, new_wallet: Address) {
         old_wallet.require_auth();
         let did_key = DataKey::PrivyDid(did.clone());
@@ -455,49 +471,84 @@ impl UserRegistryContract {
         if stored_wallet != old_wallet {
             panic!("unauthorized: old wallet does not match registered wallet");
         }
+
+        let new_wallet_did_key = DataKey::WalletDid(new_wallet.clone());
+        if let Some(existing_did) = env
+            .storage()
+            .persistent()
+            .get::<DataKey, String>(&new_wallet_did_key)
+        {
+            if existing_did != did {
+                panic!("new wallet already has a different DID linked");
+            }
+        }
+
         // Remove old reverse mapping
         env.storage()
             .persistent()
             .remove(&DataKey::WalletDid(old_wallet.clone()));
         // Update forward and reverse mappings
         env.storage().persistent().set(&did_key, &new_wallet);
+        env.storage().persistent().set(&new_wallet_did_key, &did);
         env.storage()
             .persistent()
-            .set(&DataKey::WalletDid(new_wallet.clone()), &did);
+            .extend_ttl(&did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
         env.storage()
             .persistent()
-            .extend_ttl(&did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
-        env.storage().persistent().extend_ttl(
-            &DataKey::WalletDid(new_wallet),
-            TTL_THRESHOLD,
-            TTL_EXTEND_TO,
-        );
+            .extend_ttl(&new_wallet_did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
+
+        // #982: publish so the off-chain indexer / Privy sync sees the
+        // rotation, mirroring the audit trail `register_privy_did` (#542)
+        // and every other state-changing entry point already provide.
+        env.events()
+            .publish((symbol_short!("did_upd"),), (old_wallet, new_wallet, did));
     }
 
     /// Admin recovery: reassign a DID mapping to a new wallet.
-    /// Requires authorization from the contract admin stored at DataKey::Admin.
+    ///
+    /// Issue #982: fallback for when the old wallet can no longer sign
+    /// (lost key, compromised device) and `update_privy_did`'s owner-auth
+    /// path is unavailable. Requires authorization from the contract admin
+    /// stored at `DataKey::Admin` instead of the old wallet.
+    ///
+    /// Same duplicate-wallet guard as `update_privy_did`: admin recovery
+    /// must not silently overwrite another wallet's existing DID link
+    /// either, so this is rejected exactly the same way here.
     pub fn recover_privy_did(env: Env, did: String, new_wallet: Address) {
         let admin = Self::require_admin(&env);
         admin.require_auth();
         let did_key = DataKey::PrivyDid(did.clone());
+
+        let new_wallet_did_key = DataKey::WalletDid(new_wallet.clone());
+        if let Some(existing_did) = env
+            .storage()
+            .persistent()
+            .get::<DataKey, String>(&new_wallet_did_key)
+        {
+            if existing_did != did {
+                panic!("new wallet already has a different DID linked");
+            }
+        }
+
         // Remove old reverse mapping if present
         if let Some(old_wallet) = env.storage().persistent().get::<DataKey, Address>(&did_key) {
             env.storage()
                 .persistent()
                 .remove(&DataKey::WalletDid(old_wallet));
         }
         env.storage().persistent().set(&did_key, &new_wallet);
+        env.storage().persistent().set(&new_wallet_did_key, &did);
         env.storage()
             .persistent()
-            .set(&DataKey::WalletDid(new_wallet.clone()), &did);
+            .extend_ttl(&did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
         env.storage()
             .persistent()
-            .extend_ttl(&did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
-        env.storage().persistent().extend_ttl(
-            &DataKey::WalletDid(new_wallet),
-            TTL_THRESHOLD,
-            TTL_EXTEND_TO,
-        );
+            .extend_ttl(&new_wallet_did_key, TTL_THRESHOLD, TTL_EXTEND_TO);
+
+        // #982: publish so admin-recovery events are auditable off-chain,
+        // same as the owner-initiated `update_privy_did` rotation above.
+        env.events()
+            .publish((symbol_short!("did_rec"),), (admin, new_wallet, did));
     }
 
     /// Get the wallet address registered for a Privy DID.

contracts/contracts/user_registry/src/test.rs

@@ -146,6 +146,65 @@ fn test_update_privy_did_success() {
     assert_eq!(client.get_wallet_for_did(&did), new_wallet);
 }
 
+/// Verify that `update_privy_did` publishes an event recording the rotation,
+/// so an off-chain indexer can pick it up the same way it does for
+/// `register_privy_did`'s `did_reg` event.
+#[test]
+fn test_update_privy_did_publishes_event() {
+    let (env, client, signing_key) = setup();
+    let old_wallet = Address::generate(&env);
+    let new_wallet = Address::generate(&env);
+    let did = String::from_str(&env, "did:privy:eventuser");
+    let signature = sign_did_link(&env, &signing_key, &did, &old_wallet);
+    client.register_privy_did(&did, &old_wallet, &signature);
+
+    client.update_privy_did(&did, &old_wallet, &new_wallet);
+
+    let events = env.events().all();
+    assert!(
+        events.iter().any(|e| e.topics.first().is_some()),
+        "update_privy_did must publish an event so off-chain systems can sync the rotation"
+    );
+}
+
+/// Verify that `update_privy_did` refuses to move a DID onto a wallet that
+/// already has a *different* DID linked, so it never silently overwrites
+/// that other DID's `WalletDid` reverse mapping.
+#[test]
+#[ignore = "contract panics are non-unwinding under Soroban v20 testutils and abort the test process"]
+fn test_update_privy_did_rejects_wallet_with_different_did_linked() {
+    let (env, client, signing_key) = setup();
+    let wallet_a = Address::generate(&env);
+    let wallet_b = Address::generate(&env);
+
+    let did_a = String::from_str(&env, "did:privy:rotate_a");
+    let sig_a = sign_did_link(&env, &signing_key, &did_a, &wallet_a);
+    client.register_privy_did(&did_a, &wallet_a, &sig_a);
+
+    let did_b = String::from_str(&env, "did:privy:rotate_b");
+    let sig_b = sign_did_link(&env, &signing_key, &did_b, &wallet_b);
+    client.register_privy_did(&did_b, &wallet_b, &sig_b);
+
+    // Attempt to rotate did_a onto wallet_b, which already has did_b linked.
+    let result = client.try_update_privy_did(&did_a, &wallet_a, &wallet_b);
+    assert!(
+        result.is_err(),
+        "moving a DID onto a wallet that already has a different DID linked must be rejected"
+    );
+
+    // Both original mappings must be untouched.
+    assert_eq!(
+        client.get_wallet_for_did(&did_a),
+        wallet_a,
+        "did_a's forward mapping must be unchanged after the rejected update"
+    );
+    assert_eq!(
+        client.get_did_for_wallet(&wallet_b),
+        did_b,
+        "wallet_b's reverse mapping must still point to did_b, not have been overwritten"
+    );
+}
+
 /// Verify that updating a DID mapping with the wrong old wallet is rejected.
 #[test]
 #[ignore = "contract panics are non-unwinding under Soroban v20 testutils and abort the test process"]
@@ -177,6 +236,54 @@ fn test_recover_privy_did_as_admin() {
     assert_eq!(client.get_wallet_for_did(&did), new_wallet);
 }
 
+/// Verify that `recover_privy_did` publishes an event recording the
+/// admin-initiated recovery.
+#[test]
+fn test_recover_privy_did_publishes_event() {
+    let (env, client, signing_key) = setup();
+    let old_wallet = Address::generate(&env);
+    let new_wallet = Address::generate(&env);
+    let did = String::from_str(&env, "did:privy:recovereventuser");
+    let signature = sign_did_link(&env, &signing_key, &did, &old_wallet);
+    client.register_privy_did(&did, &old_wallet, &signature);
+
+    client.recover_privy_did(&did, &new_wallet);
+
+    let events = env.events().all();
+    assert!(
+        events.iter().any(|e| e.topics.first().is_some()),
+        "recover_privy_did must publish an event so off-chain systems can sync the recovery"
+    );
+}
+
+/// Verify that `recover_privy_did` refuses to move a DID onto a wallet that
+/// already has a *different* DID linked — the same guard as
+/// `update_privy_did`, but exercised through the admin-recovery path.
+#[test]
+#[ignore = "contract panics are non-unwinding under Soroban v20 testutils and abort the test process"]
+fn test_recover_privy_did_rejects_wallet_with_different_did_linked() {
+    let (env, client, signing_key) = setup();
+    let wallet_a = Address::generate(&env);
+    let wallet_b = Address::generate(&env);
+
+    let did_a = String::from_str(&env, "did:privy:recover_a");
+    let sig_a = sign_did_link(&env, &signing_key, &did_a, &wallet_a);
+    client.register_privy_did(&did_a, &wallet_a, &sig_a);
+
+    let did_b = String::from_str(&env, "did:privy:recover_b");
+    let sig_b = sign_did_link(&env, &signing_key, &did_b, &wallet_b);
+    client.register_privy_did(&did_b, &wallet_b, &sig_b);
+
+    let result = client.try_recover_privy_did(&did_a, &wallet_b);
+    assert!(
+        result.is_err(),
+        "admin recovery must not move a DID onto a wallet that already has a different DID linked"
+    );
+
+    assert_eq!(client.get_wallet_for_did(&did_a), wallet_a);
+    assert_eq!(client.get_did_for_wallet(&wallet_b), did_b);
+}
+
 /// Verify that querying an unregistered DID returns an error.
 #[test]
 #[ignore = "contract panics are non-unwinding under Soroban v20 testutils and abort the test process"]