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 Phase 3
- [Critical/AC7.4]: Add async integration test for submit_recovery_override
using httpmock::MockServer. Test verifies that submit_recovery_override
POSTs to plc.directory, fetches updated audit log, fetches updated DID
document, and persists cached PLC log and DID doc to Keychain via
IdentityStore. Test is marked #[ignore] to avoid socket binding in
sandboxed environments.
- [Important]: Add HTTP status check after DID document fetch in
submit_recovery_override. Previously, the code did not check resp.status()
before calling .json(). Now returns NetworkError if status is not success.
- [Important]: Remove stale comment "(in later phases)" from pattern
documentation. Recovery override is now implemented, not deferred.
- [Minor]: Verify IdentityStore error messages are clear. Both store_plc_log
and store_did_doc error handlers already include clear messages:
"Failed to update cached log" and "Failed to update cached DID doc"
with the underlying error, so callers can distinguish Keychain errors
from network errors even though both use NetworkError variant.
+171
-6
+171
-6
apps/identity-wallet/src-tauri/src/recovery.rs
+171
-6
apps/identity-wallet/src-tauri/src/recovery.rs
···
1
1
// pattern: Mixed (Functional Core types + Imperative Shell commands)
2
2
//
3
3
// Functional Core: Types and error enums for recovery override operations
4
-
// Imperative Shell: Recovery override building and submission commands (in later phases)
4
+
// Imperative Shell: Recovery override building and submission commands
5
5
6
6
use crate::claim::{ChangeType, ClaimResult, OpDiff, ServiceChange};
7
7
use crate::identity_store::IdentityStore;
···
367
367
// 3. Re-fetch the DID document (it should now reflect the recovered state).
368
368
// Use the raw plc.directory endpoint, not the audit log.
369
369
let did_doc_url = format!("{}/{}", pds_client.plc_directory_url(), did);
370
-
let did_doc: serde_json::Value = pds_client
370
+
let resp = pds_client
371
371
.client()
372
372
.get(&did_doc_url)
373
373
.send()
374
374
.await
375
375
.map_err(|e| RecoveryError::NetworkError {
376
376
message: format!("Failed to fetch DID document: {e}"),
377
-
})?
378
-
.json()
379
-
.await
380
-
.map_err(|e| RecoveryError::NetworkError {
377
+
})?;
378
+
379
+
if !resp.status().is_success() {
380
+
return Err(RecoveryError::NetworkError {
381
+
message: format!("DID document fetch returned {}", resp.status()),
382
+
});
383
+
}
384
+
385
+
let did_doc: serde_json::Value =
386
+
resp.json().await.map_err(|e| RecoveryError::NetworkError {
381
387
message: format!("Failed to parse DID document: {e}"),
382
388
})?;
383
389
···
920
926
921
927
// Verify the diff is included
922
928
assert!(json.get("diff").is_some(), "diff should be present");
929
+
}
930
+
931
+
/// AC7.4: submit_recovery_override POSTs to plc.directory and updates cached log
932
+
/// Uses httpmock::MockServer to verify the submission flow.
933
+
#[tokio::test]
934
+
#[ignore] // Requires socket binding; ignore in sandboxed environments
935
+
async fn test_ac7_4_submit_recovery_override() {
936
+
use httpmock::prelude::*;
937
+
938
+
let did = "did:plc:ac74submit";
939
+
940
+
// Setup identity with device key
941
+
let store = IdentityStore;
942
+
let _ = store.add_identity(did);
943
+
for suffix in [
944
+
"device-key",
945
+
"device-key-pub",
946
+
"device-key-app-label",
947
+
"did-doc",
948
+
"plc-log",
949
+
"oauth-tokens",
950
+
] {
951
+
let _ = crate::keychain::delete_item(&format!("{did}:{suffix}"));
952
+
}
953
+
let device_pub = store
954
+
.get_or_create_device_key(did)
955
+
.expect("device key generation failed");
956
+
957
+
// Start mock server
958
+
let mock_server = MockServer::start();
959
+
let client = PdsClient::new_for_test(mock_server.base_url());
960
+
961
+
// Generate a test genesis operation
962
+
let device_priv_bytes =
963
+
crate::keychain::get_item(&format!("{did}:device-key")).expect("device key retrieval");
964
+
let device_priv_array: [u8; 32] =
965
+
device_priv_bytes.try_into().expect("device key 32 bytes");
966
+
let rotation_key = crypto::generate_p256_keypair().expect("rotation key generation");
967
+
968
+
let genesis_op = crypto::build_did_plc_genesis_op(
969
+
&rotation_key.key_id,
970
+
&crypto::DidKeyUri(device_pub.key_id.clone()),
971
+
&device_priv_array,
972
+
"test.bsky.social",
973
+
"https://pds.test",
974
+
)
975
+
.expect("build genesis op");
976
+
977
+
let genesis_operation: serde_json::Value =
978
+
serde_json::from_str(&genesis_op.signed_op_json).expect("parse genesis op json");
979
+
980
+
// Create a recovery operation (restored state matches genesis)
981
+
use std::collections::BTreeMap;
982
+
let mut verification_methods = BTreeMap::new();
983
+
verification_methods.insert(
984
+
"atproto".to_string(),
985
+
crypto::DidKeyUri(device_pub.key_id.clone()).0,
986
+
);
987
+
988
+
let recovery_op = crypto::build_did_plc_rotation_op(
989
+
"bafy_genesis",
990
+
genesis_operation
991
+
.get("rotationKeys")
992
+
.and_then(|v| v.as_array())
993
+
.map(|arr| {
994
+
arr.iter()
995
+
.filter_map(|v| v.as_str().map(String::from))
996
+
.collect()
997
+
})
998
+
.unwrap_or_default(),
999
+
verification_methods,
1000
+
vec![],
1001
+
BTreeMap::new(),
1002
+
|data: &[u8]| -> Result<Vec<u8>, crypto::CryptoError> {
1003
+
use p256::ecdsa::signature::Signer;
1004
+
use p256::ecdsa::{Signature, SigningKey};
1005
+
let signing_key = SigningKey::from_slice(&device_priv_array)
1006
+
.map_err(|_| crypto::CryptoError::KeyGeneration("Invalid key".into()))?;
1007
+
let signature: Signature = signing_key.sign(data);
1008
+
let signature = signature.normalize_s().unwrap_or(signature);
1009
+
Ok(signature.to_bytes().to_vec())
1010
+
},
1011
+
)
1012
+
.expect("build recovery op");
1013
+
1014
+
let recovery_signed_op_value: serde_json::Value =
1015
+
serde_json::from_str(&recovery_op.signed_op_json).expect("parse recovery op json");
1016
+
1017
+
// Updated audit log (after recovery operation is applied)
1018
+
let updated_audit_log_json = serde_json::json!([
1019
+
{
1020
+
"did": did,
1021
+
"cid": "bafy_genesis",
1022
+
"createdAt": "2026-03-29T00:00:00Z",
1023
+
"nullified": false,
1024
+
"operation": genesis_operation
1025
+
}
1026
+
]);
1027
+
1028
+
// DID document reflecting recovered state
1029
+
let recovered_did_doc = serde_json::json!({
1030
+
"id": did,
1031
+
"verificationMethod": [],
1032
+
"service": [
1033
+
{
1034
+
"id": "#atproto_pds",
1035
+
"type": "AtprotoPersonalDataServer",
1036
+
"serviceEndpoint": "https://pds.test"
1037
+
}
1038
+
]
1039
+
});
1040
+
1041
+
// Setup mock expectations:
1042
+
// 1. POST /{did} - submit recovery operation
1043
+
mock_server.mock(|when, then| {
1044
+
when.method(POST).path(format!("/{did}"));
1045
+
then.status(200).json_body(serde_json::json!({}));
1046
+
});
1047
+
1048
+
// 2. GET /{did}/log/audit - fetch updated audit log
1049
+
mock_server.mock(|when, then| {
1050
+
when.method(GET).path(format!("/{did}/log/audit"));
1051
+
then.status(200)
1052
+
.header("content-type", "application/json")
1053
+
.json_body(updated_audit_log_json.clone());
1054
+
});
1055
+
1056
+
// 3. GET /{did} - fetch updated DID document
1057
+
mock_server.mock(|when, then| {
1058
+
when.method(GET).path(format!("/{did}"));
1059
+
then.status(200)
1060
+
.header("content-type", "application/json")
1061
+
.json_body(recovered_did_doc.clone());
1062
+
});
1063
+
1064
+
// Execute submit_recovery_override
1065
+
let result = submit_recovery_override(&client, did, &recovery_signed_op_value)
1066
+
.await
1067
+
.expect("submit_recovery_override should succeed");
1068
+
1069
+
// Verify the cache was updated with the new audit log
1070
+
let cached_log = store.get_plc_log(did).expect("get_plc_log should succeed");
1071
+
assert!(
1072
+
cached_log.is_some(),
1073
+
"PLC log should be cached after submission"
1074
+
);
1075
+
1076
+
// Verify the DID document was stored
1077
+
let cached_did_doc = store.get_did_doc(did).expect("get_did_doc should succeed");
1078
+
assert!(
1079
+
cached_did_doc.is_some(),
1080
+
"DID document should be cached after submission"
1081
+
);
1082
+
1083
+
// Verify the result contains the updated DID doc
1084
+
assert_eq!(
1085
+
result.updated_did_doc, recovered_did_doc,
1086
+
"Result should contain the recovered DID document"
1087
+
);
923
1088
}
924
1089
}