fix(identity-wallet): address PR #67 review feedback

+27 -120

1 changed file

expand all

apps

identity-wallet

src-tauri

src

recovery.rs

+27 -120

apps/identity-wallet/src-tauri/src/recovery.rs

··· 51 51 /// Identifies the fork point — the last legitimate operation before unauthorized changes began. 52 52 /// 53 53 /// Walks backward through the audit log from the target unauthorized operation CID. 54 - /// For multiple sequential unauthorized ops (AC7.7), returns the earliest fork point 54 + /// For multiple sequential unauthorized ops, returns the earliest fork point 55 55 /// (the last device-key-signed op before the first unauthorized op in the sequence). 56 56 /// 57 57 /// Returns `(fork_point_entry, pre_unauthorized_state)` where: ··· 75 75 } 76 76 77 77 // Walk backward from the operation BEFORE the unauthorized one to find the 78 - // last operation signed by the device key. This handles AC7.7: if multiple 79 - // unauthorized ops are in sequence, we skip past all of them to find the 80 - // earliest fork point. 78 + // last operation signed by the device key. If multiple unauthorized ops are 79 + // in sequence, we skip past all of them to find the earliest fork point. 81 80 for i in (0..target_idx).rev() { 82 81 let entry = &audit_log[i]; 83 82 let op_json = ··· 156 155 /// operation before the unauthorized change), builds a PLC rotation op that 157 156 /// restores the pre-unauthorized state, and signs it with the per-DID device key. 158 157 /// 159 - /// For multiple sequential unauthorized ops (AC7.7), targets the earliest fork point. 158 + /// For multiple sequential unauthorized ops, targets the earliest fork point. 160 159 pub async fn build_recovery_override( 161 160 pds_client: &PdsClient, 162 161 did: &str, ··· 358 357 359 358 store 360 359 .store_plc_log(did, &updated_log) 361 - .map_err(|e| RecoveryError::NetworkError { 360 + .map_err(|e| RecoveryError::SigningFailed { 362 361 message: format!("Failed to cache updated PLC log in Keychain: {e}"), 363 362 })?; 364 363 ··· 387 386 388 387 store 389 388 .store_did_doc(did, &serde_json::to_string(&did_doc).unwrap_or_default()) 390 - .map_err(|e| RecoveryError::NetworkError { 389 + .map_err(|e| RecoveryError::SigningFailed { 391 390 message: format!("Failed to cache updated DID document in Keychain: {e}"), 392 391 })?; 393 392 ··· 730 729 assert!(matches!(result, Err(RecoveryError::SigningFailed { .. }))); 731 730 } 732 731 733 - /// AC7.1: build_op_diff includes fork-point CID as prev 732 + /// build_op_diff includes fork-point CID as prev 734 733 #[test] 735 - fn test_ac7_1_build_op_diff_includes_fork_cid() { 734 + fn test_build_op_diff_includes_fork_cid() { 736 735 let device_key = crypto::generate_p256_keypair().expect("device key gen"); 737 736 let rotation_key = crypto::generate_p256_keypair().expect("rotation key gen"); 738 737 ··· 751 750 ) 752 751 .expect("verify genesis op"); 753 752 754 - // AC7.1: build_op_diff should include the fork-point CID as prev 753 + // build_op_diff should include the fork-point CID as prev 755 754 let diff = build_op_diff(&verified, "bafy_genesis"); 756 755 assert_eq!( 757 756 diff.prev_cid.as_deref(), ··· 760 759 ); 761 760 } 762 761 763 - /// AC7.2: build_op_diff restores fork-point rotationKeys and services 762 + /// build_op_diff restores fork-point rotationKeys and services 764 763 #[test] 765 - fn test_ac7_2_build_op_diff_restores_keys_and_services() { 764 + fn test_build_op_diff_restores_keys_and_services() { 766 765 let device_key = crypto::generate_p256_keypair().expect("device key gen"); 767 766 let rotation_key = crypto::generate_p256_keypair().expect("rotation key gen"); 768 767 ··· 781 780 ) 782 781 .expect("verify genesis op"); 783 782 784 - // AC7.2: OpDiff should show what's being restored 783 + // OpDiff should show what's being restored 785 784 let diff = build_op_diff(&verified, "bafy_genesis"); 786 785 787 786 // Genesis has rotation_keys, so added_keys should reflect the fork-point state ··· 806 805 } 807 806 } 808 807 809 - /// AC7.5: Recovery window check rejects expired operations 810 - #[test] 811 - fn test_ac7_5_recovery_window_rejects_expired() { 812 - let expired_time = Utc::now() - Duration::hours(73); 813 - let expired_timestamp = expired_time.to_rfc3339(); 814 - 815 - let result = check_recovery_window(&expired_timestamp); 816 - assert!( 817 - matches!(result, Err(RecoveryError::RecoveryWindowExpired)), 818 - "Should reject operations older than 72 hours" 819 - ); 820 - } 821 - 822 - /// AC7.7: find_fork_point handles multiple unauthorized ops correctly 823 - #[test] 824 - fn test_ac7_7_fork_point_with_multiple_unauthorized_ops() { 825 - let device_key = crypto::generate_p256_keypair().expect("device key gen"); 826 - let rotation_key = crypto::generate_p256_keypair().expect("rotation key gen"); 827 - 828 - // Build genesis op signed by device key 829 - let genesis_op = crypto::build_did_plc_genesis_op( 830 - &rotation_key.key_id, 831 - &device_key.key_id, 832 - &device_key.private_key_bytes, 833 - "test.bsky.social", 834 - "https://pds.test", 835 - ) 836 - .expect("build genesis op"); 837 - 838 - let genesis_operation: serde_json::Value = 839 - serde_json::from_str(&genesis_op.signed_op_json).expect("parse op"); 840 - 841 - // Create two unauthorized ops (not signed by device key) 842 - let attacker1 = crypto::generate_p256_keypair().expect("attacker1 gen"); 843 - let unauth_op1 = serde_json::json!({ 844 - "type": "plc_operation", 845 - "prev": "bafy_genesis", 846 - "rotationKeys": [attacker1.key_id.0.as_str()], 847 - "verificationMethods": {}, 848 - "services": {}, 849 - "alsoKnownAs": [], 850 - "sig": "fake_sig_1" 851 - }); 852 - 853 - let attacker2 = crypto::generate_p256_keypair().expect("attacker2 gen"); 854 - let unauth_op2 = serde_json::json!({ 855 - "type": "plc_operation", 856 - "prev": "bafy_unauth1", 857 - "rotationKeys": [attacker2.key_id.0.as_str()], 858 - "verificationMethods": {}, 859 - "services": {}, 860 - "alsoKnownAs": [], 861 - "sig": "fake_sig_2" 862 - }); 863 - 864 - let audit_log_json = serde_json::json!([ 865 - { 866 - "did": "did:plc:test", 867 - "cid": "bafy_genesis", 868 - "createdAt": "2026-03-29T00:00:00Z", 869 - "nullified": false, 870 - "operation": genesis_operation 871 - }, 872 - { 873 - "did": "did:plc:test", 874 - "cid": "bafy_unauth1", 875 - "createdAt": "2026-03-29T01:00:00Z", 876 - "nullified": false, 877 - "operation": unauth_op1 878 - }, 879 - { 880 - "did": "did:plc:test", 881 - "cid": "bafy_unauth2", 882 - "createdAt": "2026-03-29T02:00:00Z", 883 - "nullified": false, 884 - "operation": unauth_op2 885 - } 886 - ]); 887 - 888 - let audit_log_str = serde_json::to_string(&audit_log_json).expect("serialize"); 889 - let audit_log = crypto::parse_audit_log(&audit_log_str).expect("parse audit log"); 890 - 891 - // AC7.7: When targeting the second unauthorized op, should find genesis (earliest fork point) 892 - let (fork_entry, _) = find_fork_point(&audit_log, "bafy_unauth2", &device_key.key_id) 893 - .expect("find_fork_point succeeded"); 894 - 895 - assert_eq!( 896 - fork_entry.cid, "bafy_genesis", 897 - "Should find earliest fork point (genesis), not first unauthorized op" 898 - ); 899 - } 900 - 901 - /// AC7.3: build_recovery_override returns a SignedRecoveryOp that can be verified with device key. 808 + /// build_recovery_override returns a SignedRecoveryOp that can be verified with device key. 902 809 /// Sets up an identity with IdentityStore, generates real keys and signed operations, 903 810 /// starts a httpmock::MockServer serving an audit log with genesis + unauthorized op, 904 811 /// calls build_recovery_override with PdsClient pointed at the mock server, 905 812 /// and verifies the returned SignedRecoveryOp signature and diff integrity. 906 813 /// 907 814 /// This test requires socket binding which is blocked in sandboxed environments. 908 - /// Run with: cargo test -p identity-wallet test_ac7_3_build_recovery_override_signs_with_device_key -- --ignored 815 + /// Run with: cargo test -p identity-wallet test_build_recovery_override_signs_with_device_key -- --ignored 909 816 #[tokio::test] 910 817 #[ignore] // Requires socket binding; ignore in sandboxed environments 911 - async fn test_ac7_3_build_recovery_override_signs_with_device_key() { 818 + async fn test_build_recovery_override_signs_with_device_key() { 912 819 use httpmock::prelude::*; 913 820 914 821 let did = "did:plc:ac73build"; ··· 991 898 .await 992 899 .expect("build_recovery_override should succeed"); 993 900 994 - // Verify AC7.1: diff.prev_cid is the fork point CID (genesis) 901 + // Verify diff.prev_cid is the fork point CID (genesis) 995 902 assert_eq!( 996 903 signed_recovery.diff.prev_cid, 997 904 Some("bafy_genesis".to_string()), 998 - "AC7.1: prev_cid should be the fork point (genesis) CID" 905 + "prev_cid should be the fork point (genesis) CID" 999 906 ); 1000 907 1001 - // Verify AC7.2: diff.added_keys contains the fork-point rotation keys 908 + // Verify diff.added_keys contains the fork-point rotation keys 1002 909 assert!( 1003 910 !signed_recovery.diff.added_keys.is_empty(), 1004 - "AC7.2: added_keys should contain rotation keys from fork point" 911 + "added_keys should contain rotation keys from fork point" 1005 912 ); 1006 913 assert!( 1007 914 signed_recovery 1008 915 .diff 1009 916 .added_keys 1010 917 .contains(&rotation_key.key_id.0), 1011 - "AC7.2: rotation_key should be in added_keys" 918 + "rotation_key should be in added_keys" 1012 919 ); 1013 920 1014 - // Verify AC7.3: signed_op can be verified via crypto::verify_plc_operation with device key 921 + // Verify signed_op can be verified via crypto::verify_plc_operation with device key 1015 922 let signed_op_json = 1016 923 serde_json::to_string(&signed_recovery.signed_op).expect("serialize signed op to JSON"); 1017 924 let device_key_uri = crypto::DidKeyUri(device_pub.key_id.clone()); ··· 1019 926 crypto::verify_plc_operation(&signed_op_json, std::slice::from_ref(&device_key_uri)); 1020 927 assert!( 1021 928 verification_result.is_ok(), 1022 - "AC7.3: Recovery operation must be verifiable with device key; got: {:?}", 929 + "Recovery operation must be verifiable with device key; got: {:?}", 1023 930 verification_result.err() 1024 931 ); 1025 932 } 1026 933 1027 - /// AC7.4: SignedRecoveryOp serializes correctly with camelCase 934 + /// SignedRecoveryOp serializes correctly with camelCase 1028 935 #[test] 1029 - fn test_ac7_4_signed_recovery_op_serializes_camel_case() { 936 + fn test_signed_recovery_op_serializes_camel_case() { 1030 937 let signed_op = SignedRecoveryOp { 1031 938 diff: OpDiff { 1032 939 added_keys: vec!["did:key:z6MkhaXgBZDvotzL".to_string()], ··· 1052 959 assert!(json.get("diff").is_some(), "diff should be present"); 1053 960 } 1054 961 1055 - /// AC7.4: submit_recovery_override POSTs to plc.directory and updates cached log 962 + /// submit_recovery_override POSTs to plc.directory and updates cached log 1056 963 /// Uses httpmock::MockServer to verify the submission flow. 1057 964 /// 1058 965 /// This test requires socket binding which is blocked in sandboxed environments. 1059 - /// Run with: cargo test -p identity-wallet test_ac7_4_submit_recovery_override -- --ignored 966 + /// Run with: cargo test -p identity-wallet test_submit_recovery_override -- --ignored 1060 967 #[tokio::test] 1061 968 #[ignore] // Requires socket binding; ignore in sandboxed environments 1062 - async fn test_ac7_4_submit_recovery_override() { 969 + async fn test_submit_recovery_override() { 1063 970 use httpmock::prelude::*; 1064 971 1065 972 let did = "did:plc:ac74submit";

Configure Feed

Configure Feed