add new fields: alias_name and last_used

[BillyWooo]

Add comprehensive passkey management improvements including user-friendly
alias names, usage tracking, and rename functionality.

Changes:

• Add omni_account, alias_name, and last_used fields to PasskeyRecord
• Update add_passkey() to accept optional alias_name (defaults to credential_id)
• Modify list_passkeys() to return (alias_name, created_at, last_used) tuples
• Add rename_passkey_alias() for updating passkey display names
• Add update_last_used() to track passkey authentication usage
• Create new omni_renamePasskeyAliasName RPC endpoint with auth verification
• Integrate automatic last_used updates in passkey authentication flow
• Update all tests and integration tests for new signature

Add cleanup methods for unit test leftover
Improve passkey storage

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
heima-aa-demo-app	Ignored			Nov 7, 2025 8:53pm

[claude]

Claude finished @BillyWooo's task —— View job

---

PR Review: Passkey Management Improvements

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze core passkey storage changes
• Review RPC endpoint implementations
• Check authentication integration
• Evaluate test coverage
• Assess security implications
• Provide comprehensive feedback

---

Review Summary

This PR introduces comprehensive passkey management improvements including user-friendly aliases, usage tracking, and rename functionality. The implementation is generally well-structured and follows good security practices. Here's my detailed analysis:

✅ Strengths

1. Security Best Practices

   • Proper authentication verification before sensitive operations
   • Robust input validation and error handling
   • Secure challenge-response mechanisms maintained
   • No exposure of sensitive data in logs or responses

2. Code Quality

   • Clean separation of concerns between storage and RPC layers
   • Consistent error handling patterns
   • Good test coverage with both unit and integration tests
   • Proper use of Rust's type system for safety

3. Feature Implementation

   • Well-designed alias system that defaults to credential_id
   • Automatic last_used timestamp tracking
   • Backward-compatible API changes

⚠️ Areas for Improvement

Critical Issues

1. Potential Timing Attack in Authentication (verify_auth.rs:447-456)

// Update the last_used timestamp for this passkey
passkey_storage
    .update_last_used(&omni_account, &passkey_data.credential_id)
    .map_err(|_| {
        // Log the error but don't fail authentication if timestamp update fails
        tracing::warn!(
            "Failed to update last_used timestamp for passkey {}",
            passkey_data.credential_id
        );
    })
    .ok();

Issue: The code logs credential_id on failure, which could leak information about valid credentials to attackers analyzing logs.

Recommendation: Remove credential_id from the warning log:

tracing::warn!("Failed to update last_used timestamp for passkey");

Performance Concerns

2. Inefficient Database Iteration (passkey.rs:138-195)

The list_passkeys() method iterates through ALL keys in the storage namespace to find matches for a specific account. This is inefficient and won't scale well.

Current Implementation: O(n) scan of all passkey records

let iter = db.prefix_iterator(storage_prefix);
for item in iter { /* check each record */ }

Recommendations:

• Add an index mapping omni_account -> Vec<credential_id>
• Or restructure storage keys to enable prefix-based queries by account
• Consider pagination for large result sets

Code Quality Issues

3. Error Handling Inconsistency (rename_passkey_alias.rs:53-56)

if !passkey_storage.exists_passkey(&omni_account, &params.credential_id) {
    error!("Passkey not found for this account and credential");
    return Err(DetailedError::internal_error("Passkey not found").to_rpc_error());
}

Issue: Uses internal_error for a client error scenario.

Recommendation: Use a more appropriate error type:

return Err(DetailedError::not_found("passkey", "Passkey not found").to_rpc_error());

4. Redundant Validation Logic

The passkey existence check in rename_passkey_alias is redundant since rename_passkey_alias() already handles this case and returns ValidationError.

Minor Issues

5. Magic Number (verify_auth.rs:400)

if auth_data_bytes.len() < 37 {

Recommendation: Define a constant:

const MIN_AUTH_DATA_LENGTH: usize = 37; // 32 bytes RP ID hash + 1 byte flags + 4 bytes counter

6. Missing Input Validation

• No length limits on alias_name parameter
• No validation of alias_name content (special characters, etc.)

🧪 Test Coverage Assessment

Excellent coverage with:

• Unit tests for all storage operations
• Integration tests for complete workflows
• Edge cases (duplicates, expiration, concurrent access)

Missing coverage:

• RPC endpoint error scenarios
• Alias name validation edge cases
• Large-scale performance scenarios

📋 Recommendations

High Priority

1. Fix timing attack vulnerability in logging
2. Add alias_name length validation (suggest max 64 characters)
3. Optimize list_passkeys() performance

Medium Priority

4. Improve error classification in rename endpoint
5. Add alias_name content validation
6. Remove redundant existence checks

Low Priority

7. Add constants for magic numbers
8. Add performance tests for large passkey collections

🔒 Security Assessment

Overall security posture is strong, but the information leakage in error logging should be addressed before merge.

🚀 Performance Impact

The current implementation will have O(n) performance degradation as the number of passkeys grows. For production use with many users, the storage pattern should be optimized.

---

Verdict: This is a solid implementation with good security practices. The main concerns are around performance scalability and a minor information leakage issue. I recommend addressing the critical logging issue before merge, with performance optimization as a follow-up task.

[Kailai-Wang]

Looks fine

[Kailai-Wang]

Hmm where is this field used?

[BillyWooo]

It's not used yet actually. I just added it there in case somewhere needs it. Current map structure doesn't have any directly info about omni_account.