An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
fix(identity-wallet): address code review feedback for recovery override
- C1+I1: Add build_recovery_override integration test verifying AC7.3 signing with device key.
This test sets up an identity with IdentityStore, generates real keys and signed operations,
starts a httpmock::MockServer serving an audit log with genesis + unauthorized op,
calls build_recovery_override with PdsClient pointed at the mock server,
and verifies the returned SignedRecoveryOp can be verified with device key (AC7.3),
includes the fork point CID as prev (AC7.1), and contains fork-point rotation keys (AC7.2).
Test is #[ignore] to skip in sandboxed environments as it requires socket binding.
- C2: Add clear documentation comment to test_ac7_4_submit_recovery_override explaining:
- This test requires the --ignored flag to run
- It requires socket binding which is blocked in sandboxed environments
- Provides exact cargo test command for running it
- M1: Remove vestigial #[allow(dead_code)] annotations from find_fork_point and check_recovery_window.
Both functions are actively called by build_recovery_override and do not need the annotation.
- M2: Fix error messages to clearly indicate Keychain failures vs network errors:
- Change "Failed to update cached log" to "Failed to cache updated PLC log in Keychain"
- Change "Failed to update cached DID doc" to "Failed to cache updated DID document in Keychain"
These changes clarify that storage failures are Keychain operations, not network operations.
+137
-4
+137
-4
apps/identity-wallet/src-tauri/src/recovery.rs
+137
-4
apps/identity-wallet/src-tauri/src/recovery.rs
···
57
57
/// Returns `(fork_point_entry, pre_unauthorized_state)` where:
58
58
/// - `fork_point_entry` is the last legitimate AuditEntry (its CID becomes the `prev` for the counter-op)
59
59
/// - `pre_unauthorized_state` is the VerifiedPlcOp representing the state to restore
60
-
#[allow(dead_code)]
61
60
pub(crate) fn find_fork_point(
62
61
audit_log: &[AuditEntry],
63
62
unauthorized_op_cid: &str,
···
135
134
///
136
135
/// Returns `Ok(())` if recovery is still possible, or `Err(RecoveryWindowExpired)` if
137
136
/// the 72-hour deadline has passed.
138
-
#[allow(dead_code)]
139
137
pub(crate) fn check_recovery_window(unauthorized_op_created_at: &str) -> Result<(), RecoveryError> {
140
138
let op_time = DateTime::parse_from_rfc3339(unauthorized_op_created_at)
141
139
.map_err(|e| RecoveryError::SigningFailed {
···
361
359
store
362
360
.store_plc_log(did, &updated_log)
363
361
.map_err(|e| RecoveryError::NetworkError {
364
-
message: format!("Failed to update cached log: {e}"),
362
+
message: format!("Failed to cache updated PLC log in Keychain: {e}"),
365
363
})?;
366
364
367
365
// 3. Re-fetch the DID document (it should now reflect the recovered state).
···
390
388
store
391
389
.store_did_doc(did, &serde_json::to_string(&did_doc).unwrap_or_default())
392
390
.map_err(|e| RecoveryError::NetworkError {
393
-
message: format!("Failed to update cached DID doc: {e}"),
391
+
message: format!("Failed to cache updated DID document in Keychain: {e}"),
394
392
})?;
395
393
396
394
Ok(ClaimResult {
···
900
898
);
901
899
}
902
900
901
+
/// AC7.3: build_recovery_override returns a SignedRecoveryOp that can be verified with device key.
902
+
/// Sets up an identity with IdentityStore, generates real keys and signed operations,
903
+
/// starts a httpmock::MockServer serving an audit log with genesis + unauthorized op,
904
+
/// calls build_recovery_override with PdsClient pointed at the mock server,
905
+
/// and verifies the returned SignedRecoveryOp signature and diff integrity.
906
+
///
907
+
/// 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
909
+
#[tokio::test]
910
+
#[ignore] // Requires socket binding; ignore in sandboxed environments
911
+
async fn test_ac7_3_build_recovery_override_signs_with_device_key() {
912
+
use httpmock::prelude::*;
913
+
914
+
let did = "did:plc:ac73build";
915
+
916
+
// Setup identity with IdentityStore (pattern from plc_monitor.rs)
917
+
let store = IdentityStore;
918
+
let _ = store.add_identity(did);
919
+
for suffix in [
920
+
"device-key",
921
+
"device-key-pub",
922
+
"device-key-app-label",
923
+
"did-doc",
924
+
"plc-log",
925
+
"oauth-tokens",
926
+
] {
927
+
let _ = crate::keychain::delete_item(&format!("{did}:{suffix}"));
928
+
}
929
+
let device_pub = store
930
+
.get_or_create_device_key(did)
931
+
.expect("device key generation failed");
932
+
let device_priv_bytes: [u8; 32] = crate::keychain::get_item(&format!("{did}:device-key"))
933
+
.expect("device key retrieval")
934
+
.try_into()
935
+
.expect("device key 32 bytes");
936
+
937
+
// Generate rotation key for genesis
938
+
let rotation_key = crypto::generate_p256_keypair().expect("rotation key generation");
939
+
940
+
// Build real genesis operation signed by device key
941
+
let genesis_op = crypto::build_did_plc_genesis_op(
942
+
&rotation_key.key_id,
943
+
&crypto::DidKeyUri(device_pub.key_id.clone()),
944
+
&device_priv_bytes,
945
+
"test.bsky.social",
946
+
"https://pds.test",
947
+
)
948
+
.expect("build genesis op");
949
+
950
+
let genesis_operation: serde_json::Value =
951
+
serde_json::from_str(&genesis_op.signed_op_json).expect("parse genesis op json");
952
+
953
+
// Create unauthorized operation (signed by attacker, not device key)
954
+
let attacker_key = crypto::generate_p256_keypair().expect("attacker key generation");
955
+
let unauth_operation = serde_json::json!({
956
+
"type": "plc_operation",
957
+
"prev": "bafy_genesis",
958
+
"rotationKeys": [attacker_key.key_id.0.as_str()],
959
+
"verificationMethods": {},
960
+
"services": {},
961
+
"alsoKnownAs": [],
962
+
"sig": "fake_attacker_signature"
963
+
});
964
+
965
+
// Build audit log JSON
966
+
let audit_log_json = serde_json::json!([
967
+
{
968
+
"did": did,
969
+
"cid": "bafy_genesis",
970
+
"createdAt": "2026-03-29T00:00:00Z",
971
+
"nullified": false,
972
+
"operation": genesis_operation
973
+
},
974
+
{
975
+
"did": did,
976
+
"cid": "bafy_unauthorized",
977
+
"createdAt": "2026-03-29T01:00:00Z",
978
+
"nullified": false,
979
+
"operation": unauth_operation
980
+
}
981
+
]);
982
+
983
+
// Setup mock server
984
+
let mock_server = MockServer::start();
985
+
let client = PdsClient::new_for_test(mock_server.base_url());
986
+
987
+
// Mock GET /{did}/log/audit — returns audit log with genesis + unauthorized op
988
+
mock_server.mock(|when, then| {
989
+
when.method(GET).path(format!("/{did}/log/audit"));
990
+
then.status(200)
991
+
.header("content-type", "application/json")
992
+
.json_body(audit_log_json.clone());
993
+
});
994
+
995
+
// Execute build_recovery_override
996
+
let signed_recovery = build_recovery_override(&client, did, "bafy_unauthorized")
997
+
.await
998
+
.expect("build_recovery_override should succeed");
999
+
1000
+
// Verify AC7.1: diff.prev_cid is the fork point CID (genesis)
1001
+
assert_eq!(
1002
+
signed_recovery.diff.prev_cid,
1003
+
Some("bafy_genesis".to_string()),
1004
+
"AC7.1: prev_cid should be the fork point (genesis) CID"
1005
+
);
1006
+
1007
+
// Verify AC7.2: diff.added_keys contains the fork-point rotation keys
1008
+
assert!(
1009
+
!signed_recovery.diff.added_keys.is_empty(),
1010
+
"AC7.2: added_keys should contain rotation keys from fork point"
1011
+
);
1012
+
assert!(
1013
+
signed_recovery
1014
+
.diff
1015
+
.added_keys
1016
+
.contains(&rotation_key.key_id.0),
1017
+
"AC7.2: rotation_key should be in added_keys"
1018
+
);
1019
+
1020
+
// Verify AC7.3: signed_op can be verified via crypto::verify_plc_operation with device key
1021
+
let signed_op_json =
1022
+
serde_json::to_string(&signed_recovery.signed_op).expect("serialize signed op to JSON");
1023
+
let device_key_uri = crypto::DidKeyUri(device_pub.key_id.clone());
1024
+
let verification_result =
1025
+
crypto::verify_plc_operation(&signed_op_json, std::slice::from_ref(&device_key_uri));
1026
+
assert!(
1027
+
verification_result.is_ok(),
1028
+
"AC7.3: Recovery operation must be verifiable with device key; got: {:?}",
1029
+
verification_result.err()
1030
+
);
1031
+
}
1032
+
903
1033
/// AC7.4: SignedRecoveryOp serializes correctly with camelCase
904
1034
#[test]
905
1035
fn test_ac7_4_signed_recovery_op_serializes_camel_case() {
···
930
1060
931
1061
/// AC7.4: submit_recovery_override POSTs to plc.directory and updates cached log
932
1062
/// Uses httpmock::MockServer to verify the submission flow.
1063
+
///
1064
+
/// This test requires socket binding which is blocked in sandboxed environments.
1065
+
/// Run with: cargo test -p identity-wallet test_ac7_4_submit_recovery_override -- --ignored
933
1066
#[tokio::test]
934
1067
#[ignore] // Requires socket binding; ignore in sandboxed environments
935
1068
async fn test_ac7_4_submit_recovery_override() {