fix: address code review feedback on claim flow backend

+46 -40

2 changed files

expand all

apps

identity-wallet

src-tauri

src

claim.rs

pds_client.rs

+46 -38

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

··· 552 552 state: tauri::State<'_, crate::oauth::AppState>, 553 553 _did: String, 554 554 ) -> Result<(), ClaimError> { 555 - // Acquire lock, extract Arc<OAuthClient>, and release lock before making network call 556 - let oauth_client = { 555 + // Acquire lock, extract claim state, and release lock before making network call 556 + let claim_state_copy = { 557 557 let claim_state = state.claim_state.lock().await; 558 558 let Some(claim) = claim_state.as_ref() else { 559 559 return Err(ClaimError::Unauthorized); 560 560 }; 561 - claim.pds_oauth_client.clone() 561 + claim.clone() 562 562 }; // claim_state lock released here 563 563 564 - let Some(oauth_client) = oauth_client else { 565 - return Err(ClaimError::Unauthorized); 566 - }; 567 - 568 - crate::pds_client::request_plc_operation_signature(&oauth_client) 569 - .await 570 - .map_err(|e| ClaimError::NetworkError { 571 - message: format!("request_plc_operation_signature failed: {}", e), 572 - }) 564 + request_claim_verification_impl(&claim_state_copy).await 573 565 } 574 566 575 567 /// Testable core logic for `request_claim_verification`. 576 568 /// 577 - /// Extracted to a separate function to avoid requiring Tauri's `State` in tests. 578 - #[cfg(test)] 569 + /// Extracted to a separate function to allow testing without Tauri's State. 579 570 pub(crate) async fn request_claim_verification_impl( 580 571 claim_state: &ClaimState, 581 572 ) -> Result<(), ClaimError> { ··· 604 595 #[tauri::command] 605 596 pub async fn sign_and_verify_claim( 606 597 state: tauri::State<'_, crate::oauth::AppState>, 607 - device_key_id: String, 598 + did: String, 608 599 token: String, 609 600 ) -> Result<VerifiedClaimOp, ClaimError> { 610 601 // Acquire lock, extract required data, and release lock before making network calls ··· 626 617 ) 627 618 }; // claim_state lock released here 628 619 620 + // Step 2: Look up the device key ID using IdentityStore 621 + let device_key = crate::identity_store::IdentityStore 622 + .get_or_create_device_key(&did) 623 + .map_err(|e| ClaimError::NetworkError { 624 + message: format!("failed to get device key: {}", e), 625 + })?; 626 + 629 627 let (verified_op, signed_op_json) = sign_and_verify_claim_impl( 630 628 pds_client_ref, 631 629 &oauth_client_ref, 632 630 &claim_did, 633 631 &claim_did_doc, 634 - &device_key_id, 632 + &device_key.key_id, 635 633 &token, 636 634 ) 637 635 .await?; ··· 980 978 message: format!("failed to store PLC log: {}", e), 981 979 })?; 982 980 981 + // Step 4: Clear ClaimState (handled by the Tauri command caller after this function succeeds) 983 982 // Step 5: Return the updated DID document 984 983 Ok(ClaimResult { 985 984 updated_did_doc: did_doc_value, ··· 1580 1579 }, 1581 1580 ); 1582 1581 1583 - let (rotation_json, _signing_key) = build_test_rotation_op( 1582 + let (rotation_json, signing_key) = build_test_rotation_op( 1584 1583 &device_key, 1585 1584 vec![device_key.clone()], 1586 1585 services.clone(), ··· 1654 1653 let did_doc = PlcDidDocument { 1655 1654 did: "did:plc:test".to_string(), 1656 1655 also_known_as: vec!["at://alice.example.com".to_string()], 1657 - rotation_keys: vec![], 1656 + rotation_keys: vec![signing_key.clone()], 1658 1657 verification_methods: serde_json::json!({}), 1659 1658 services: HashMap::new(), 1660 1659 }; ··· 1671 1670 1672 1671 assert!( 1673 1672 result.is_ok(), 1674 - "should return Ok for valid operation with device key at rotationKeys[0]" 1673 + "expected Ok, got: {:?}", 1674 + result.err() 1675 1675 ); 1676 1676 let (verified_op, _signed_op_json) = result.unwrap(); 1677 1677 assert!( 1678 1678 verified_op.diff.added_keys.contains(&device_key), 1679 - "should have device key in added_keys" 1679 + "should have device key in added_keys, got: {:?}", 1680 + verified_op.diff.added_keys 1680 1681 ); 1681 1682 } 1682 1683 ··· 1702 1703 }, 1703 1704 ); 1704 1705 1705 - let (rotation_json, _signing_key) = 1706 + let (rotation_json, signing_key) = 1706 1707 build_test_rotation_op(&wrong_key, vec![wrong_key.clone()], services, &prev_cid); 1707 1708 1708 1709 // Mock endpoints ··· 1763 1764 let did_doc = PlcDidDocument { 1764 1765 did: "did:plc:test".to_string(), 1765 1766 also_known_as: vec!["at://alice.example.com".to_string()], 1766 - rotation_keys: vec![], 1767 + rotation_keys: vec![signing_key.clone()], 1767 1768 verification_methods: serde_json::json!({}), 1768 1769 services: HashMap::new(), 1769 1770 }; ··· 1780 1781 1781 1782 assert!( 1782 1783 matches!(result, Err(ClaimError::VerificationFailed { .. })), 1783 - "should return VerificationFailed when device key is not at rotationKeys[0]" 1784 + "should return VerificationFailed when device key is not at rotationKeys[0], got: {:?}", 1785 + result 1784 1786 ); 1785 1787 } 1786 1788 ··· 1806 1808 ); 1807 1809 1808 1810 // Build with wrong_prev 1809 - let (rotation_json, _signing_key) = 1811 + let (rotation_json, signing_key) = 1810 1812 build_test_rotation_op(&device_key, vec![device_key.clone()], services, &wrong_prev); 1811 1813 1812 1814 mock_server.mock(|when, then| { ··· 1867 1869 let did_doc = PlcDidDocument { 1868 1870 did: "did:plc:test".to_string(), 1869 1871 also_known_as: vec!["at://alice.example.com".to_string()], 1870 - rotation_keys: vec![], 1872 + rotation_keys: vec![signing_key.clone()], 1871 1873 verification_methods: serde_json::json!({}), 1872 1874 services: HashMap::new(), 1873 1875 }; ··· 1884 1886 1885 1887 assert!( 1886 1888 matches!(result, Err(ClaimError::VerificationFailed { .. })), 1887 - "should return VerificationFailed when prev doesn't match audit log" 1889 + "should return VerificationFailed when prev doesn't match audit log, got: {:?}", 1890 + result 1888 1891 ); 1889 1892 } 1890 1893 ··· 1910 1913 ); 1911 1914 1912 1915 // Build operation with only device key (missing original_key) 1913 - let (rotation_json, _signing_key) = 1916 + let (rotation_json, signing_key) = 1914 1917 build_test_rotation_op(&device_key, vec![device_key.clone()], services, &prev_cid); 1915 1918 1916 1919 mock_server.mock(|when, then| { ··· 1970 1973 let did_doc = PlcDidDocument { 1971 1974 did: "did:plc:test".to_string(), 1972 1975 also_known_as: vec!["at://alice.example.com".to_string()], 1973 - rotation_keys: vec![original_key.clone()], 1976 + rotation_keys: vec![signing_key.clone(), original_key.clone()], 1974 1977 verification_methods: serde_json::json!({}), 1975 1978 services: HashMap::new(), 1976 1979 }; ··· 1987 1990 1988 1991 assert!( 1989 1992 matches!(result, Err(ClaimError::VerificationFailed { .. })), 1990 - "should return VerificationFailed when a rotation key is removed" 1993 + "should return VerificationFailed when a rotation key is removed, got: {:?}", 1994 + result 1991 1995 ); 1992 1996 } 1993 1997 ··· 2011 2015 }, 2012 2016 ); 2013 2017 2014 - let (rotation_json, _signing_key) = 2018 + let (rotation_json, signing_key) = 2015 2019 build_test_rotation_op(&device_key, vec![device_key.clone()], services, &prev_cid); 2016 2020 2017 2021 mock_server.mock(|when, then| { ··· 2080 2084 let did_doc = PlcDidDocument { 2081 2085 did: "did:plc:test".to_string(), 2082 2086 also_known_as: vec!["at://alice.example.com".to_string()], 2083 - rotation_keys: vec![], 2087 + rotation_keys: vec![signing_key.clone()], 2084 2088 verification_methods: serde_json::json!({}), 2085 2089 services: original_services, 2086 2090 }; ··· 2097 2101 2098 2102 assert!( 2099 2103 matches!(result, Err(ClaimError::VerificationFailed { .. })), 2100 - "should return VerificationFailed when service endpoint is changed" 2104 + "should return VerificationFailed when service endpoint is changed, got: {:?}", 2105 + result 2101 2106 ); 2102 2107 } 2103 2108 ··· 2129 2134 }, 2130 2135 ); 2131 2136 2132 - let (rotation_json, _signing_key) = 2137 + let (rotation_json, signing_key) = 2133 2138 build_test_rotation_op(&device_key, vec![device_key.clone()], services, &prev_cid); 2134 2139 2135 2140 mock_server.mock(|when, then| { ··· 2198 2203 let did_doc = PlcDidDocument { 2199 2204 did: "did:plc:test".to_string(), 2200 2205 also_known_as: vec!["at://alice.example.com".to_string()], 2201 - rotation_keys: vec![], 2206 + rotation_keys: vec![signing_key.clone()], 2202 2207 verification_methods: serde_json::json!({}), 2203 2208 services: original_services, 2204 2209 }; ··· 2215 2220 2216 2221 assert!( 2217 2222 result.is_ok(), 2218 - "should succeed even with added service (benign warning)" 2223 + "should succeed even with added service (benign warning), got: {:?}", 2224 + result.err() 2219 2225 ); 2220 2226 let (verified_op, _signed_op_json) = result.unwrap(); 2221 2227 assert!( 2222 2228 !verified_op.warnings.is_empty(), 2223 - "should have warnings about added service" 2229 + "should have warnings about added service, got: {:?}", 2230 + verified_op.warnings 2224 2231 ); 2225 2232 } 2226 2233 ··· 2277 2284 let did_doc = PlcDidDocument { 2278 2285 did: "did:plc:test".to_string(), 2279 2286 also_known_as: vec!["at://alice.example.com".to_string()], 2280 - rotation_keys: vec![], 2287 + rotation_keys: vec!["did:key:zQ3signing".to_string()], 2281 2288 verification_methods: serde_json::json!({}), 2282 2289 services: HashMap::new(), 2283 2290 }; ··· 2294 2301 2295 2302 assert!( 2296 2303 matches!(result, Err(ClaimError::InvalidToken)), 2297 - "should return InvalidToken when PDS returns InvalidToken error" 2304 + "should return InvalidToken when PDS returns InvalidToken error, got: {:?}", 2305 + result 2298 2306 ); 2299 2307 } 2300 2308

-2

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

··· 141 141 /// PDS client for discovery and OAuth operations against arbitrary PDS endpoints. 142 142 /// 143 143 /// Stateless except for the HTTP client which pools connections. 144 - #[allow(dead_code)] 145 - // TODO(Phase 4): Remove #[allow(dead_code)] once Tauri commands call PdsClient methods 146 144 pub struct PdsClient { 147 145 client: Client, 148 146 plc_directory_url: String,

Configure Feed

Configure Feed