Add hardware security key verification for Bitcoin transactions#116
Conversation
- Add new authenticateWithSecurityKeyOnly() method that bypasses passkey selection UI - Only prompts for hardware security key (single device) authentication - Uses the exact username from registered hardware key (e.g. 33@nuri.com) - Prevents showing platform passkeys during transaction verification - Ensures only the registered hardware key can authorize transactions - Improved error handling for missing username/credential scenarios The implementation now correctly: 1. Checks if user has a hardware key registered (username + credential ID) 2. When verifying, uses ONLY the security key provider (no platform provider) 3. Passes the exact username to match the single device registration 4. Directly prompts for NFC/USB security key without showing passkey list 🤖 Generated with Claude Code (https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Use fetchAuthenticationOptions instead of non-existent getAuthenticationOptions - Replace non-existent PasskeyError.invalidChallenge with .serverError - Fixes compilation errors for hardware security key authentication
- Add allowedCredentials filter to only show the registered credential - Prevents showing multiple credentials when hardware key has many stored - Uses the stored credential ID from registration to filter the list - Add helper methods to manage stored credentials - Support both USB and NFC transports for hardware keys This ensures that when the user taps their hardware key, they only see the specific credential that was registered for this app, not all credentials stored on the key. 🤖 Generated with Claude Code (https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix nil userID crash in hardware key authentication - Hardware security keys often don't have a userID/userHandle - Make userID optional throughout the verification flow - Properly handle nil userID in logging and API requests The crash was caused by force-unwrapping credential.userID which is nil for many hardware security key assertions. Now safely handles this case with optional chaining. 🤖 Generated with Claude Code (https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughReplaced Buy Bitcoin flow with a WKWebView to Mercuryo and updated navigation to present it. Added a hardware security key–only authentication path and integrated key verification into the send-confirmation flow. Tweaked Receive to copy address and show a toast before opening Buy. Removed testnet-specific buy label variant. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ReceiveView
participant Clipboard
participant Toast
participant Nav as BitcoinViewNavigation
participant BuyView as BuyBitcoinView (WKWebView)
User->>ReceiveView: Tap "Buy Bitcoin"
alt Valid address
ReceiveView->>Clipboard: Copy BTC address
Clipboard-->>ReceiveView: Success
ReceiveView->>Toast: Show "Address copied!"
else Invalid/missing address
ReceiveView->>Toast: Show "Address copied!" (skipped copy)
end
Note over ReceiveView,Toast: Toast visible ~2s, then hides
ReceiveView->>Nav: isReceiveViewPresented = false
ReceiveView->>Nav: isBuyBitcoinPresented = true
Nav->>BuyView: Present sheet
BuyView->>BuyView: Load https://exchange.mercuryo.io/ in WKWebView
sequenceDiagram
autonumber
actor User
participant Confirm as ConfirmTransactionView
participant PasskeySvc as PasskeyAuthenticationService
participant AS as AuthenticationServices
participant Server
User->>Confirm: Tap primary button
alt HW key registered && not verified
Confirm->>User: Alert to verify with hardware key
User->>Confirm: Continue
Confirm->>AS: Request presentation anchor
Confirm->>PasskeySvc: authenticateWithSecurityKeyOnly(username, anchor)
PasskeySvc->>Server: Fetch auth options
PasskeySvc->>AS: Present security key assertion UI
AS-->>PasskeySvc: Assertion (or cancel/error)
PasskeySvc->>Server: Verify assertion
Server-->>PasskeySvc: Verification result
PasskeySvc-->>Confirm: (verified, username, isAnonymous)
alt Verified
Confirm->>Confirm: securityKeyVerified = true
Confirm->>Server: Send transaction
else Cancel/Error
Confirm->>Confirm: Show error or abort
end
else No HW key gating
Confirm->>Server: Send transaction
end
Confirm->>Confirm: Reset securityKeyVerified after success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @eminogrande's task —— View job Code Review: Hardware Security Key Verification for Bitcoin TransactionsTodo List:
SummaryThis PR implements hardware security key verification for Bitcoin transactions, requiring users to verify with their registered security key (YubiKey, etc.) before signing transactions. The implementation includes credential filtering, proper error handling, and clean integration with the existing transaction flow. Detailed Code Review✅ Strengths
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift (5)
250-257: Redact PII in logs (credentialId, userHandle).Good move making
userHandlenil-safe. However, logging full credential IDs and user handles is sensitive. Prefer truncation or hashing, and gate these behind DEBUG.Apply this diff to mask values in both places:
- Log.passkey.success("User selected platform credential (built-in)", metadata: [ - "credentialId": credential.credentialID.base64URLEncodedString(), - "userHandle": credential.userID?.base64URLEncodedString() ?? "none", + Log.passkey.success("User selected platform credential (built-in)", metadata: [ + "credentialId": credential.credentialID.base64URLEncodedString().prefix(10) + "...", + "userHandle": (credential.userID?.base64URLEncodedString().prefix(10) ?? "none") + "...",- Log.passkey.success("User selected security key credential (hardware key)", metadata: [ - "credentialId": credential.credentialID.base64URLEncodedString(), - "userHandle": credential.userID?.base64URLEncodedString() ?? "none", + Log.passkey.success("User selected security key credential (hardware key)", metadata: [ + "credentialId": credential.credentialID.base64URLEncodedString().prefix(10) + "...", + "userHandle": (credential.userID?.base64URLEncodedString().prefix(10) ?? "none") + "...",Also applies to: 303-309
853-864: Reduce sensitive logging in auth verification.Even at debug level, logging
credentialIdand full body increases risk. Mask identifiers; keep raw JSON logging strictly in DEBUG and consider redaction.- Log.passkey.debug("Security key verification request", metadata: [ - "credentialId": credentialIdBase64, - "authenticatorType": "securityKey", - "userHandle": credential.userID?.base64URLEncodedString() ?? "none", - "endpoint": endpoint - ]) + Log.passkey.debug("Security key verification request", metadata: [ + "credentialId": credentialIdBase64.prefix(10) + "...", + "authenticatorType": "securityKey", + "userHandle": (credential.userID?.base64URLEncodedString().prefix(10) ?? "none") + "...", + "endpoint": endpoint + ]) #if DEBUG - if let jsonString = String(data: request.httpBody!, encoding: .utf8) { + if let jsonString = String(data: request.httpBody!, encoding: .utf8) { Log.passkey.debug("Security key auth request JSON", metadata: ["body": jsonString]) } #endif
924-943: Wrap detailed clientData logging in DEBUG (it contains challenge and origin).
clientDataJSONmay include sensitive values. Gate the decoded dump behind DEBUG to avoid appearing in production logs.- if let clientDataJSON = try? JSONSerialization.jsonObject(with: credential.rawClientDataJSON, options: []) as? [String: Any] { - Log.passkey.debug("Client data JSON decoded", metadata: clientDataJSON) - } + #if DEBUG + if let clientDataJSON = try? JSONSerialization.jsonObject(with: credential.rawClientDataJSON, options: []) as? [String: Any] { + Log.passkey.debug("Client data JSON decoded", metadata: clientDataJSON) + } + #endif
771-791: Avoid logging full URLs with PII query items.
usernamein the query is PII; loggingurl.absoluteStringleaks it. Redact or log path only.- Log.network.info("Sending auth options request", metadata: ["url": url.absoluteString, "username": username ?? "any"]) + Log.network.info("Sending auth options request", metadata: [ + "path": url.path, + "hasUsername": username != nil + ])
1566-1594: Avoid fatalError() in presentationAnchor; fail gracefully.Crashing on missing window is harsh and can occur in edge cases (multi-scene transitions, backgrounded state). Prefer a soft failure and a plausible fallback anchor.
- fatalError("No window available for passkey presentation") + assertionFailure("No window available for passkey presentation") + // Return a non-nil placeholder to avoid crash; controller will likely fail and surface an error. + return UIWindow(frame: UIScreen.main.bounds)Nuri/Nuri/Sources/Views/Bitcoin/Send/ConfirmTransactionView.swift (1)
16-20: Replace verbose print statements with the app logger or gate behind DEBUG.These prints will ship to production and clutter logs. Use
Log.*or wrap prints in#if DEBUG.Example:
- print("🚀 [ConfirmTransactionView] sendTransaction() called") + Log.ui.info("[ConfirmTransactionView] sendTransaction() called")Or:
- print("✅ [ConfirmTransactionView] TransactionManager.sendTransaction() completed successfully!") + #if DEBUG + print("✅ [ConfirmTransactionView] TransactionManager.sendTransaction() completed successfully!") + #endifAlso applies to: 224-226, 373-377, 398-404, 416-421, 435-447, 454-460, 465-471, 492-499, 501-510, 512-524
🧹 Nitpick comments (18)
Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift (4)
387-393: Prefer service helpers over raw UserDefaults access (single source of truth).You added
getStoredCredentialInfo(). Use it everywhere (including UI) to avoid key drift and ease future migrations (e.g., to Keychain).
422-425: UVA set to discouraged — confirm policy for high-value actions.Setting
userVerificationPreference = .discouragedavoids PIN prompts, but reduces assurance from UVA to mere presence (touch). For funds movement, some compliance programs expect UVA (PIN on key) at least optionally.If policy allows, keep as-is. Otherwise consider a server flag to toggle between
.preferredand.requiredfor certain transaction risk tiers.
426-438: Support multiple credentials and server-provided filters.Filtering by a single locally stored credential ID is brittle. If a user re-registers or has multiple credentials on the same key, rely on the server’s
allowCredentials(if present) and fall back to local storage only if needed.Example changes:
- Extend
AuthenticationOptionsResponse:struct AuthenticationOptionsResponse: Codable { let challenge: String let timeout: Int let rpId: String let userVerification: String + let allowCredentials: [AllowedCredential]? + + struct AllowedCredential: Codable { + let type: String + let id: String + } }
- Build
allowedCredentialsfrom the response first:- if let storedCredentialId = UserDefaults.standard.string(forKey: "passkeyCredentialId"), + if let allow = authOptions.allowCredentials, !allow.isEmpty { + let descriptors: [ASAuthorizationSecurityKeyPublicKeyCredentialDescriptor] = allow.compactMap { + guard let idData = Data(base64URLEncoded: $0.id) else { return nil } + return .init(credentialID: idData, transports: [.usb, .nfc]) + } + securityKeyRequest.allowedCredentials = descriptors + } else if let storedCredentialId = UserDefaults.standard.string(forKey: "passkeyCredentialId"), let credentialIdData = Data(base64URLEncoded: storedCredentialId) {
1188-1191: Temporary authProof placeholder — track with TODO and feature flag.
authProofis a placeholder and “server doesn't validate yet”. Add a TODO and guard submission under a feature flag to prevent shipping this path inadvertently if server-side validation toggles later.Nuri/Nuri/Sources/Views/Bitcoin/Send/ConfirmTransactionView.swift (6)
55-63: Use PasskeyAuthenticationService helper instead of raw UserDefaults.Avoid duplicating key names and storage logic in the view. Leverage
getStoredCredentialInfo()for readability and future migrations (e.g., to Keychain).- let hasUsername = UserDefaults.standard.string(forKey: "passkeyUsername") != nil - let hasCredentialId = UserDefaults.standard.string(forKey: "passkeyCredentialId") != nil + let stored = PasskeyAuthenticationService.shared.getStoredCredentialInfo() + let hasUsername = stored.username != nil + let hasCredentialId = stored.credentialId != nil
46-47: Disable button while verifying security key to prevent re-entrancy.Include
isVerifyingSecurityKeyinshouldDisableButtonto avoid multiple concurrent verification attempts.- private var shouldDisableButton: Bool { - return isSending || transactionInfo == nil || transactionData == nil || isInsufficientFunds - } + private var shouldDisableButton: Bool { + return isSending || isVerifyingSecurityKey || transactionInfo == nil || transactionData == nil || isInsufficientFunds + }
257-281: Overlay blocks interaction — good. Consider haptics for prompt acknowledgment.The full-screen overlay avoids accidental taps during verification. Optionally, trigger a light haptic when the OS sheet appears.
384-397: Gating relies on local presence of both username and credentialId — potential bypass.If the credential ID wasn’t persisted (e.g., app reinstall without restore), a registered user on the server won’t be gated. Consider checking with
PasskeyAuthenticationService.checkUserExists(username:)or using the auth options endpoint to infer presence before sending.Minimal improvement without extra network trips:
- let hasUsername = UserDefaults.standard.string(forKey: "passkeyUsername") != nil - let hasCredentialId = UserDefaults.standard.string(forKey: "passkeyCredentialId") != nil + let stored = PasskeyAuthenticationService.shared.getStoredCredentialInfo() + let hasUsername = stored.username != nil + let hasCredentialId = stored.credentialId != nilIf you want stronger guarantees, I can wire a lightweight cached “has registered passkey” flag from server.
465-498: Centralize presentation anchor retrieval.This logic duplicates similar window discovery in
PasskeyAuthenticationService. Prefer a single helper (e.g.,PresentationAnchorProvider.shared.currentAnchor()) to avoid divergence.I can extract the common code into a tiny utility and update both call sites.
96-103: Guard against divide-by-zero when computing EUR rate.If
amountSatsever becomes zero,btcAmountis zero, causing a division by zero foreurRate. Add a small guard.- let btcAmount = Double(txData.amountSats) / 100_000_000 - let eurRate = txData.eurAmount / btcAmount + let btcAmount = Double(txData.amountSats) / 100_000_000 + let eurRate = btcAmount > 0 ? (txData.eurAmount / btcAmount) : 0Nuri/Nuri/Sources/Views/Bitcoin/Receive/ReceiveView.swift (2)
70-89: Streamline navigation sequencing and add UIKit import for UIPasteboard.
- The nested DispatchQueue calls work but are brittle. Prefer a single Task with async sleeps to avoid race-y nested callbacks and make cancellation easier.
- Ensure UIKit is imported since UIPasteboard lives there; otherwise this will fail to compile on some targets.
Apply this refactor within the button action to simplify flow and timing:
- Button("Buy Bitcoin") { - // Copy address to clipboard - if Self.isBitcoinAddress(address) { - UIPasteboard.general.string = address - showCopiedToast = true - - // Hide toast after 2 seconds - DispatchQueue.main.asyncAfter(deadline: .now() + 2.0) { - showCopiedToast = false - } - } - - // Close receive view first, then open buy webview - DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { - navigation.isReceiveViewPresented = false - DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { - navigation.isBuyBitcoinPresented = true - } - } - } + Button("Buy Bitcoin") { + Task { @MainActor in + if Self.isBitcoinAddress(address) { + UIPasteboard.general.string = address + showCopiedToast = true + try? await Task.sleep(nanoseconds: 2_000_000_000) + showCopiedToast = false + } + navigation.isReceiveViewPresented = false + try? await Task.sleep(nanoseconds: 800_000_000) + navigation.isBuyBitcoinPresented = true + } + }To support UIPasteboard without surprises, add this near the top of the file (outside the changed range):
import UIKitOptional UX polish (non-diff):
- Fire a success haptic and accessibility announcement after copying:
let gen = UINotificationFeedbackGenerator(); gen.notificationOccurred(.success)
UIAccessibility.post(notification: .announcement, argument: "Bitcoin address copied.")
96-117: Toast overlay works; consider safe-area awareness and reduced-motion.
- Fixed 50pt top padding can collide with status bars/notches across devices. Prefer safeAreaInset at .top or use safeAreaInset + transition.
- Respect Reduce Motion by disabling animations when UIAccessibility.isReduceMotionEnabled is true.
Example adjustment (outside diff range):
.safeAreaInset(edge: .top) { if showCopiedToast { HStack(spacing: 8) { Image(systemName: "checkmark.circle.fill").foregroundColor(.white) Text("Bitcoin address copied!").foregroundColor(.white).font(.system(size: 14, weight: .medium)) } .padding() .background(Color.black.opacity(0.8)) .cornerRadius(20) .transition(.move(edge: .top).combined(with: .opacity)) .animation(UIAccessibility.isReduceMotionEnabled ? nil : .easeInOut, value: showCopiedToast) } }Nuri/Nuri/Sources/Views/Bitcoin/BitcoinView.swift (2)
157-160: Wrap the new Buy sheet in a NavigationStack for consistency; keep explicit env injection.Other sheets use NavigationStack, so align this one. Keeping the explicit .environmentObject avoids any ordering pitfalls with environment propagation across modifiers.
Apply this localized diff:
- .sheet(isPresented: $navigation.isBuyBitcoinPresented) { - BuyBitcoinView() - .environmentObject(navigation) - } + .sheet(isPresented: $navigation.isBuyBitcoinPresented) { + NavigationStack { + BuyBitcoinView() + .environmentObject(navigation) + } + }
7-7: Unify Buy flow presentations into a single routeOur audit confirms two independent presentation flags in
BitcoinView.swift, each driving its own sheet:
isBuyViewPresented→ presentsBuyBitcoinFlowView()(legacy/Striga flow)isBuyBitcoinPresented→ presentsBuyBitcoinView()(Mercuryo flow)With both flags published separately, there’s no built-in mutual exclusion—if both are set to
truein quick succession, SwiftUI may try to show two sheets simultaneously. To prevent overlapping presentations and simplify state management, consider consolidating these into a single enum-backed route, for example:enum BuyRoute { case none case legacy case mercuryo } @Published var buyRoute: BuyRoute = .noneThen drive a single
.sheetoffbuyRoute, switching its content based on the enum case.• File: Nuri/Nuri/Sources/Views/Bitcoin/BitcoinView.swift
– Lines 7–8:
swift @Published var isBuyBitcoinPresented = false @Published var isBuyViewPresented = false
– Lines 152–159: two separate.sheet(isPresented:)modifiersThis refactor will:
- Eliminate the risk of double presentations
- Centralize buy-flow logic into one state variable
- Make future additions (e.g., new providers) easier to slot in
Nuri/Nuri/Sources/Views/Buy Bitcoin/BuyBitcoinView.swift (4)
4-18: Harden WKWebView configuration: ephemeral datastore and better UX.
- Consider nonPersistent websiteDataStore to avoid persisting third-party cookies/tracking across sessions (privacy-friendly default).
- Enable back/forward gestures for better navigation.
- Optionally set contentMode to .mobile for better layout if the site serves responsive content.
Apply this focused diff:
struct BuyBitcoinWebView: UIViewRepresentable { func makeUIView(context: Context) -> WKWebView { let configuration = WKWebViewConfiguration() configuration.preferences.javaScriptEnabled = true + configuration.websiteDataStore = .nonPersistent() let webView = WKWebView(frame: .zero, configuration: configuration) webView.navigationDelegate = context.coordinator + webView.allowsBackForwardNavigationGestures = true + if #available(iOS 16.4, *) { + webView.isInspectable = false + } if let url = URL(string: "https://exchange.mercuryo.io/") { let request = URLRequest(url: url) webView.load(request) } return webView }Note: If Mercuryo requires persistent sessions (e.g., KYC), keep the default persistent store. Otherwise, ephemeral is safer-by-default.
26-33: Add provisional navigation error handling and tighten navigation policy to the target domain.
- didFailProvisionalNavigation catches initial load failures (DNS/SSL/etc.).
- Optionally, restrict navigation to exchange.mercuryo.io and open external links in SFSafariViewController if needed to prevent unintended domain escapes.
Apply this localized diff:
class Coordinator: NSObject, WKNavigationDelegate { func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { print("✅ Mercuryo exchange loaded") } func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { print("❌ Failed to load Mercuryo: \(error)") } + + func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) { + print("❌ Provisional navigation failed: \(error)") + } + + func webView(_ webView: WKWebView, + decidePolicyFor navigationAction: WKNavigationAction, + decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { + if let host = navigationAction.request.url?.host, + host.hasSuffix("mercuryo.io") { + decisionHandler(.allow) + } else { + // Optional: open external links in Safari instead of inside the webview + decisionHandler(.cancel) + if let url = navigationAction.request.url { + UIApplication.shared.open(url) + } + } + } }Caveat: If the flow legitimately navigates to subdomains or auxiliary domains, expand the allowlist accordingly.
37-50: Dismiss action is clear; add minimal loading/error UI around the webview.Right now, failures only log to console. Consider a simple spinner and a retry affordance driven by Coordinator callbacks to improve user feedback (aligned with the PR’s “improved error handling and user feedback” goal).
Example (compact) pattern:
- Add @State private var isLoading = true and @State private var loadError: String? in BuyBitcoinView.
- Update Coordinator to flip these states via bindings (e.g., pass closures from BuyBitcoinWebView to Coordinator).
- Overlay a ProgressView while isLoading, and a small in-view error with “Retry” that calls webView.reload().
If you want, I can draft a concrete diff wiring these states end-to-end.
12-15: Prefill the destination BTC address via Mercuryo URL parametersIt’s now confirmed that Mercuryo’s widget supports an
addressquery parameter (along with a required cryptographicsignature) to prefill the user’s deposit address, which eliminates the need to rely on the clipboard and reduces input errors. You can enhance the existing loader in BuyBitcoinView.swift by constructing the URL with the following parameters:• widget_id – your Mercuryo widget identifier
• address – the user’s BTC address to receive funds
• merchant_transaction_id – a unique ID for this transaction (optional but recommended)
• signature – HMAC-SHA512 (or as specified) over (address + widget_secret + ip_address + merchant_transaction_id), generated on your backendExample updated snippet:
// Assume `btcAddress`, `widgetID`, `transactionID` are available, // and `fetchSignature(...)` retrieves a valid signature from your backend. let baseURL = "https://exchange.mercuryo.io/" var components = URLComponents(string: baseURL)! components.queryItems = [ URLQueryItem(name: "widget_id", value: widgetID), URLQueryItem(name: "address", value: btcAddress), URLQueryItem(name: "merchant_transaction_id", value: transactionID), URLQueryItem(name: "signature", value: fetchSignature(address: btcAddress, widgetID: widgetID, transactionID: transactionID)) ] if let url = components.url { let request = URLRequest(url: url) webView.load(request) }• Ensure your backend implements Mercuryo’s signature protocol (typically HMAC-SHA512) and that the widget’s “Address Prefill” feature is enabled in your Mercuryo dashboard.
• By passing these parameters, the widget will automatically populate the destination address field, improving UX and reducing clipboard–related pitfalls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
Nuri/Nuri/Sources/Common/NetworkConfiguration.swift(0 hunks)Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift(7 hunks)Nuri/Nuri/Sources/Views/Bitcoin/BitcoinView.swift(1 hunks)Nuri/Nuri/Sources/Views/Bitcoin/Receive/ReceiveView.swift(2 hunks)Nuri/Nuri/Sources/Views/Bitcoin/Send/ConfirmTransactionView.swift(7 hunks)Nuri/Nuri/Sources/Views/Buy Bitcoin/BuyBitcoinView.swift(1 hunks)
💤 Files with no reviewable changes (1)
- Nuri/Nuri/Sources/Common/NetworkConfiguration.swift
🧰 Additional context used
🧬 Code graph analysis (3)
Nuri/Nuri/Sources/Views/Bitcoin/Send/ConfirmTransactionView.swift (3)
Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift (2)
authenticateWithSecurityKeyOnly(395-478)presentationAnchor(1567-1597)Nuri/Nuri/Sources/Views/Onboarding/OnboardingWireframe.swift (1)
presentationAnchor(125-127)Nuri/Nuri/Sources/Views/Onboarding/Login/LoginViewModel.swift (1)
presentationAnchor(113-115)
Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift (1)
Nuri/Nuri/Sources/Common/Logger.swift (2)
warning(79-88)success(68-77)
Nuri/Nuri/Sources/Views/Buy Bitcoin/BuyBitcoinView.swift (4)
Nuri/Nuri/Sources/Views/Card/StrigaCardWebView.swift (2)
makeUIView(203-265)webView(280-292)Nuri/Nuri/Sources/Views/Card/CardConsentWebView.swift (3)
makeUIView(21-232)webView(263-310)webView(312-314)Nuri/Nuri/Sources/Views/Card/HostedCardView.swift (3)
makeUIView(12-34)webView(49-65)webView(67-69)Nuri/Nuri/Sources/Design System/UI Components/NuriHeader.swift (1)
logo(48-68)
🔇 Additional comments (6)
Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift (1)
395-478: Solid hardware-key-only path and credential filtering.Clean, direct flow: server-driven
rpId, security-key-only request,allowedCredentialsfilter, and strict type check. This is exactly what we need to bypass platform passkeys.Nuri/Nuri/Sources/Views/Bitcoin/Send/ConfirmTransactionView.swift (3)
31-35: Nice, explicit UI state for hardware-key verification.Clear separation of alert, overlay, and “verified” flag makes the flow easy to reason about.
246-256: Good UX: clear alert copy and explicit verify action.The alert-driven entry to verification makes the requirement obvious without surprising the user.
500-510: Re-entering send flow from verification risks double logs; functional flow is fine.Calling
sendTransaction()after settingsecurityKeyVerified = trueis sound. With the button disabled while verifying (see earlier comment), re-entrancy is controlled.Nuri/Nuri/Sources/Views/Bitcoin/Receive/ReceiveView.swift (1)
8-8: State flag for toast is appropriate and minimal.The dedicated boolean keeps the toast rendering simple and predictable.
Nuri/Nuri/Sources/Views/Buy Bitcoin/BuyBitcoinView.swift (1)
53-56: Preview setup looks correct with environment object.No issues.
| // Helper method to clear stored passkey credentials (useful for debugging) | ||
| func clearStoredCredentials() { | ||
| Log.passkey.info("Clearing stored passkey credentials") | ||
| UserDefaults.standard.removeObject(forKey: "passkeyUsername") | ||
| UserDefaults.standard.removeObject(forKey: "passkeyCredentialId") | ||
| UserDefaults.standard.removeObject(forKey: "passkeyIsAnonymous") | ||
| UserDefaults.standard.removeObject(forKey: "passkeyUserEmail") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not store passkey identifiers in UserDefaults; move to Keychain-backed store.
passkeyUsername, passkeyCredentialId, and passkeyUserEmail are PII and should not live in UserDefaults. Use Keychain (kSecClassGenericPassword) and wrap with a small CredentialsStore for atomic get/set/clear.
I can provide a minimal Keychain helper and a migration that reads old values from UserDefaults, writes to Keychain, then clears the old keys. Want a patch?
🤖 Prompt for AI Agents
In Nuri/Nuri/Sources/Services/PasskeyAuthenticationService.swift around lines
378–385, the helper currently clears passkeyUsername, passkeyCredentialId,
passkeyIsAnonymous, and passkeyUserEmail from UserDefaults but three of those
(username, credentialId, userEmail) are PII and must not be stored in
UserDefaults; implement a Keychain-backed CredentialsStore (use
kSecClassGenericPassword) with atomic get/set/clear methods and replace all
reads/writes to those three keys to use the store, add a migration routine that
on first run reads existing values from UserDefaults, writes them into Keychain,
then removes the old UserDefaults keys, and update clearStoredCredentials to
clear the credentials from the Keychain-backed store (and only remove the
UserDefaults keys as part of migration cleanup).
1 participant
Summary
Key Features
✅ Hardware key required for Bitcoin sends when registered
✅ Direct NFC/USB security key authentication (bypasses passkey selection)
✅ Filters to show only the registered credential
✅ Handles nil userID for hardware keys (fixes crash)
✅ Uses exact username from registration (e.g., 33@nuri.com)
Technical Implementation
authenticateWithSecurityKeyOnly()method for hardware-only authallowedCredentialsTesting
🤖 Generated with Claude Code (https://claude.ai/code)
Summary by CodeRabbit
New Features
Changes