An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.
fix(MM-144): address PR review feedback on account creation and keychain handling
CRITICAL fix:
- Prevent orphaned private key on token-storage failure: add keychain::delete_item()
and best-effort cleanup when device-token or session-token write fails after relay
201 success. This prevents a situation where the account is created on relay but
the device has no tokens due to local Keychain failure.
IMPORTANT fixes:
- Map keychain failures to new CreateAccountError::KeychainError (not Unknown),
giving users accurate error message: 'Couldn't save credentials to your device.
Try again.' instead of misleading 'Couldn't reach the server.'
- Distinguish UNKNOWN from NETWORK_ERROR: NETWORK_ERROR is genuine connectivity
failure, UNKNOWN is for relay-reachable errors (e.g., unrecognized 409 subcodes).
Frontend routes each to different screens with appropriate messages.
- Fix 404 comment to clarify relay returns 404 for both invalid and expired codes.
- Add unit test for 409 subcode dispatch table to prevent typos in
CLAIM_CODE_REDEEMED/ACCOUNT_EXISTS/HANDLE_TAKEN mapping.
- Fix HandleScreen handle validation: change from non-empty string to ATProto
regex /^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$/ to prevent relying on
relay validation for invalid formats.
- Update CreateMobileAccountResponse comment to note that relay returns 5 fields
but only 3 are captured (additional fields ignored by serde).
- Add console.error for unexpected nextStep values in submit flow.
- Update ipc.ts with KEYCHAIN_ERROR variant and clean up JSDoc.
- Add Cargo.toml comment explaining rustls-tls requirement (no OpenSSL on iOS).
- Update CLAUDE.md to clarify keypair generation behavior: fresh keypair per
attempt (with best-effort Keychain cleanup on failure), not retry reuse.
TESTS:
- Add CreateAccountError::KeychainError serialization test
- Add CreateAccountError::Unknown serialization test
- Add 409 subcode dispatch table test (verifies correct mapping of
CLAIM_CODE_REDEEMED, ACCOUNT_EXISTS, HANDLE_TAKEN, unknown subcodes)
All tests pass: cargo test --workspace (231 tests)
Clippy: no warnings
Formatting: passes
Frontend: pnpm build succeeds, svelte-check passes
authored by
Tangled
a3c19f15
8c687de9
+141
-18
+1
-1
apps/identity-wallet/CLAUDE.md
+1
-1
apps/identity-wallet/CLAUDE.md
···
140
140
- **`tauri` and `tauri-build` declared locally**: These crates are not in `[workspace.dependencies]` because no other workspace crate uses them. `serde` and `serde_json` use `{ workspace = true }` per the standard workspace pattern.
141
141
- **`src-tauri/.cargo/config.toml` committed**: Overrides `CC`, `AR`, and `linker` for iOS and macOS-host targets to use Xcode's unwrapped clang instead of the Nix cc-wrapper. Without this, Nix's clang wrapper injects macOS-specific flags (`-mmacos-version-min`, macOS sysroot) that are incompatible with iOS cross-compilation. See the Troubleshooting section for the full explanation.
142
142
- **Compile-time relay URL**: `http.rs` uses `#[cfg(debug_assertions)]` to switch between localhost:8080 (debug) and relay.ezpds.com (release). No runtime configuration needed for the base URL.
143
-
- **Keychain-before-network ordering**: `create_account` stores the private key in Keychain before POSTing to the relay -- if the network call fails, the key is already persisted and can be reused on retry.
143
+
- **Keychain-before-network ordering**: `create_account` stores the private key in Keychain **before** POSTing to the relay. This ensures that if the network call fails, the private key is already persisted. On each new account creation attempt (whether first attempt or retry), a fresh keypair is generated and stored, overwriting any prior key. This is safe because the relay is stateless per claim code; each attempt with a new keypair is treated as a new account creation request.
144
144
- **reqwest with rustls-tls**: Uses `default-features = false` + `rustls-tls` to avoid linking OpenSSL. On iOS, rustls handles TLS natively without additional system deps.
145
145
146
146
## Invariants
+2
apps/identity-wallet/src-tauri/Cargo.toml
+2
apps/identity-wallet/src-tauri/Cargo.toml
···
18
18
# serde/serde_json are in workspace.dependencies (root Cargo.toml lines 30-32)
19
19
serde = { workspace = true }
20
20
serde_json = { workspace = true }
21
+
# reqwest with default-features = false + rustls-tls: iOS has no OpenSSL; rustls is the only
22
+
# option for TLS without adding system dependencies. default-features includes OpenSSL support.
21
23
reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }
22
24
security-framework = "3"
23
25
thiserror = { workspace = true }
+10
-1
apps/identity-wallet/src-tauri/src/keychain.rs
+10
-1
apps/identity-wallet/src-tauri/src/keychain.rs
···
4
4
//! service `"ezpds-identity-wallet"`. Use the `SERVICE` constant
5
5
//! to ensure consistency.
6
6
7
-
use security_framework::passwords::{get_generic_password, set_generic_password};
7
+
use security_framework::passwords::{
8
+
delete_generic_password, get_generic_password, set_generic_password,
9
+
};
8
10
9
11
pub const SERVICE: &str = "ezpds-identity-wallet";
10
12
···
27
29
pub fn get_item(account: &str) -> Result<Vec<u8>, KeychainError> {
28
30
get_generic_password(SERVICE, account).map_err(KeychainError::Security)
29
31
}
32
+
33
+
/// Delete an item from the Keychain by account name.
34
+
///
35
+
/// Returns `Ok(())` on successful deletion, or `Err` if the item doesn't exist.
36
+
pub fn delete_item(account: &str) -> Result<(), KeychainError> {
37
+
delete_generic_password(SERVICE, account).map_err(KeychainError::Security)
38
+
}
+113
-13
apps/identity-wallet/src-tauri/src/lib.rs
+113
-13
apps/identity-wallet/src-tauri/src/lib.rs
···
20
20
}
21
21
22
22
/// Successful 201 response from the relay.
23
+
///
24
+
/// The relay returns additional fields (account_id, device_id) which are
25
+
/// silently ignored by serde's default behavior. This struct captures only
26
+
/// the three fields needed by the client.
23
27
#[derive(Deserialize)]
24
28
#[serde(rename_all = "camelCase")]
25
29
struct CreateMobileAccountResponse {
···
63
67
EmailTaken,
64
68
#[error("handle already taken")]
65
69
HandleTaken,
70
+
#[error("keychain storage failed")]
71
+
KeychainError,
66
72
#[error("network error: {message}")]
67
73
NetworkError { message: String },
68
74
#[error("unknown error: {message}")]
···
88
94
89
95
// 2. Store private key bytes in Keychain before any network call.
90
96
// private_key_bytes is Zeroizing<[u8; 32]>; deref to &[u8] via AsRef.
91
-
keychain::store_item("device-private-key", keypair.private_key_bytes.as_ref()).map_err(
92
-
|e| CreateAccountError::Unknown {
93
-
message: e.to_string(),
94
-
},
95
-
)?;
97
+
keychain::store_item("device-private-key", keypair.private_key_bytes.as_ref())
98
+
.map_err(|_| CreateAccountError::KeychainError)?;
96
99
97
100
// 3. POST to relay.
98
101
let req = CreateMobileAccountRequest {
···
120
123
})?;
121
124
122
125
// 5. Store tokens in Keychain.
123
-
keychain::store_item("device-token", body.device_token.as_bytes()).map_err(|e| {
124
-
CreateAccountError::Unknown {
125
-
message: e.to_string(),
126
-
}
126
+
// If either token write fails, clean up the private key (best-effort) to avoid
127
+
// orphaning a key on the relay with no tokens to access it.
128
+
keychain::store_item("device-token", body.device_token.as_bytes()).map_err(|_| {
129
+
// Best-effort cleanup: ignore deletion errors.
130
+
let _ = keychain::delete_item("device-private-key");
131
+
CreateAccountError::KeychainError
127
132
})?;
128
-
keychain::store_item("session-token", body.session_token.as_bytes()).map_err(|e| {
129
-
CreateAccountError::Unknown {
130
-
message: e.to_string(),
131
-
}
133
+
134
+
keychain::store_item("session-token", body.session_token.as_bytes()).map_err(|_| {
135
+
// Best-effort cleanup: ignore deletion errors.
136
+
let _ = keychain::delete_item("device-private-key");
137
+
CreateAccountError::KeychainError
132
138
})?;
133
139
134
140
Ok(CreateAccountResult {
···
137
143
} else {
138
144
// 6. Map relay error codes to typed variants.
139
145
match status.as_u16() {
146
+
// 404: Relay returns this for both invalid (never-existed) and expired claim codes.
147
+
// The frontend cannot distinguish them, so we map both to ExpiredCode.
140
148
404 => Err(CreateAccountError::ExpiredCode),
141
149
409 => {
142
150
let envelope: RelayErrorEnvelope =
···
240
248
let json = serde_json::to_value(&err).unwrap();
241
249
assert_eq!(json["code"], "NETWORK_ERROR");
242
250
assert_eq!(json["message"], "Connection timeout");
251
+
}
252
+
253
+
// -- AC3.6: CreateAccountError::KeychainError serialization --
254
+
#[test]
255
+
fn error_keychain_error_serializes_correctly() {
256
+
let err = CreateAccountError::KeychainError;
257
+
let json = serde_json::to_value(&err).unwrap();
258
+
assert_eq!(json["code"], "KEYCHAIN_ERROR");
259
+
}
260
+
261
+
// -- AC3.7: CreateAccountError::Unknown serialization --
262
+
#[test]
263
+
fn error_unknown_serializes_correctly() {
264
+
let err = CreateAccountError::Unknown {
265
+
message: "Unexpected relay response".into(),
266
+
};
267
+
let json = serde_json::to_value(&err).unwrap();
268
+
assert_eq!(json["code"], "UNKNOWN");
269
+
assert_eq!(json["message"], "Unexpected relay response");
270
+
}
271
+
272
+
// -- 409 subcode dispatch table --
273
+
#[test]
274
+
fn error_409_dispatch_maps_subcodes_correctly() {
275
+
// Test CLAIM_CODE_REDEEMED subcode
276
+
let envelope = RelayErrorEnvelope {
277
+
error: RelayErrorBody {
278
+
code: "CLAIM_CODE_REDEEMED".to_string(),
279
+
},
280
+
};
281
+
let err = match envelope.error.code.as_str() {
282
+
"CLAIM_CODE_REDEEMED" => CreateAccountError::RedeemedCode,
283
+
"ACCOUNT_EXISTS" => CreateAccountError::EmailTaken,
284
+
"HANDLE_TAKEN" => CreateAccountError::HandleTaken,
285
+
other => CreateAccountError::Unknown {
286
+
message: format!("409: {other}"),
287
+
},
288
+
};
289
+
let json = serde_json::to_value(&err).unwrap();
290
+
assert_eq!(json["code"], "REDEEMED_CODE");
291
+
292
+
// Test ACCOUNT_EXISTS subcode
293
+
let envelope = RelayErrorEnvelope {
294
+
error: RelayErrorBody {
295
+
code: "ACCOUNT_EXISTS".to_string(),
296
+
},
297
+
};
298
+
let err = match envelope.error.code.as_str() {
299
+
"CLAIM_CODE_REDEEMED" => CreateAccountError::RedeemedCode,
300
+
"ACCOUNT_EXISTS" => CreateAccountError::EmailTaken,
301
+
"HANDLE_TAKEN" => CreateAccountError::HandleTaken,
302
+
other => CreateAccountError::Unknown {
303
+
message: format!("409: {other}"),
304
+
},
305
+
};
306
+
let json = serde_json::to_value(&err).unwrap();
307
+
assert_eq!(json["code"], "EMAIL_TAKEN");
308
+
309
+
// Test HANDLE_TAKEN subcode
310
+
let envelope = RelayErrorEnvelope {
311
+
error: RelayErrorBody {
312
+
code: "HANDLE_TAKEN".to_string(),
313
+
},
314
+
};
315
+
let err = match envelope.error.code.as_str() {
316
+
"CLAIM_CODE_REDEEMED" => CreateAccountError::RedeemedCode,
317
+
"ACCOUNT_EXISTS" => CreateAccountError::EmailTaken,
318
+
"HANDLE_TAKEN" => CreateAccountError::HandleTaken,
319
+
other => CreateAccountError::Unknown {
320
+
message: format!("409: {other}"),
321
+
},
322
+
};
323
+
let json = serde_json::to_value(&err).unwrap();
324
+
assert_eq!(json["code"], "HANDLE_TAKEN");
325
+
326
+
// Test unknown subcode (falls through to Unknown)
327
+
let envelope = RelayErrorEnvelope {
328
+
error: RelayErrorBody {
329
+
code: "UNKNOWN_SUBCODE".to_string(),
330
+
},
331
+
};
332
+
let err = match envelope.error.code.as_str() {
333
+
"CLAIM_CODE_REDEEMED" => CreateAccountError::RedeemedCode,
334
+
"ACCOUNT_EXISTS" => CreateAccountError::EmailTaken,
335
+
"HANDLE_TAKEN" => CreateAccountError::HandleTaken,
336
+
other => CreateAccountError::Unknown {
337
+
message: format!("409: {other}"),
338
+
},
339
+
};
340
+
let json = serde_json::to_value(&err).unwrap();
341
+
assert_eq!(json["code"], "UNKNOWN");
342
+
assert!(json["message"].as_str().unwrap().contains("409:"));
243
343
}
244
344
}
+4
-1
apps/identity-wallet/src/lib/components/onboarding/HandleScreen.svelte
+4
-1
apps/identity-wallet/src/lib/components/onboarding/HandleScreen.svelte
···
9
9
error?: string;
10
10
} = $props();
11
11
12
-
let isValid = $derived(value.trim().length > 0);
12
+
// ATProto handle validation: alphanumeric start/end, dots/hyphens/underscores allowed in middle.
13
+
// Minimum 1 character, maximum typically 63 (per DNS labels).
14
+
const handleRegex = /^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$/;
15
+
let isValid = $derived(handleRegex.test(value.trim()));
13
16
</script>
14
17
15
18
<div class="screen">
+2
-1
apps/identity-wallet/src/lib/ipc.ts
+2
-1
apps/identity-wallet/src/lib/ipc.ts
···
20
20
* Error returned by the `create_account` Rust command.
21
21
*
22
22
* Serialized as `{ code: "EXPIRED_CODE" }` etc. by the Rust backend.
23
-
* The `message` field is present only on NETWORK_ERROR and UNKNOWN variants.
23
+
* The `message` field is present only on variants that include it in their Rust definition.
24
24
* This is a pure data shape used for error handling.
25
25
*/
26
26
export type CreateAccountError = {
···
29
29
| 'REDEEMED_CODE'
30
30
| 'EMAIL_TAKEN'
31
31
| 'HANDLE_TAKEN'
32
+
| 'KEYCHAIN_ERROR'
32
33
| 'NETWORK_ERROR'
33
34
| 'UNKNOWN';
34
35
message?: string;
+9
-1
apps/identity-wallet/src/routes/+page.svelte
+9
-1
apps/identity-wallet/src/routes/+page.svelte
···
61
61
step = 'did_ceremony';
62
62
} else {
63
63
// Unexpected nextStep value — treat as success and advance anyway.
64
+
console.error('Unexpected nextStep:', result.nextStep);
64
65
step = 'did_ceremony';
65
66
}
66
67
} catch (raw: unknown) {
···
97
98
errors.handle = 'That handle is taken. Please choose another.';
98
99
step = 'handle';
99
100
break;
101
+
case 'KEYCHAIN_ERROR':
102
+
errors.handle = "Couldn't save credentials to your device. Try again.";
103
+
step = 'handle';
104
+
break;
100
105
case 'NETWORK_ERROR':
106
+
errors.handle = "Couldn't reach the server. Check your connection.";
107
+
step = 'handle';
108
+
break;
101
109
case 'UNKNOWN':
102
110
default:
103
-
errors.handle = "Couldn't reach the server. Check your connection.";
111
+
errors.handle = 'Something went wrong. Please try again.';
104
112
step = 'handle';
105
113
break;
106
114
}