Skip to content

Explicitly evaluate biometrics before wallet confirmation - #112

Open
CHUKEPC wants to merge 3 commits into
TelegramMessenger:masterfrom
CHUKEPC:fix/wallet-touch-id-passcode
Open

CHUKEPC wants to merge 3 commits into
TelegramMessenger:masterfrom
CHUKEPC:fix/wallet-touch-id-passcode

Conversation

@CHUKEPC

@CHUKEPC CHUKEPC commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Key change

  • Add an explicit evaluatePolicy(.deviceOwnerAuthenticationWithBiometrics) step before the protected wallet-key read in WalletUIIntegration.swift. Only proceed to authenticateWalletBiometrics after that explicit evaluation succeeds.
  • Master already creates a new LAContext for each attempt and the protected key read authenticates through the OS. Per-attempt context creation is preserved, not introduced here; the explicit zero reuse interval merely states the documented default.
  • Successful biometric evaluation alone does not authorize an operation. authenticateWalletBiometrics must still return a valid resource-scoped PasscodeCore session from an OS-protected key read. Cancellation or a failed protected read returns the existing cancelled result; the context is invalidated and completion is delivered on the main queue.
  • Keep biometric enrollment, keychain access control, and validation-alert behavior identical to master. The PR does not change enabling/disabling Touch ID or alert handling in Wallet settings, Business Hours, Passport, or other forms.

Reason

The original report concerned the visibility of the wallet confirmation prompt. This PR makes biometric evaluation an explicit step in the confirmation flow instead of relying solely on authentication initiated by the protected-key read. It does not fix context reuse or change the OS key protection: master already used a new context and protected authentication on each attempt. This is a confirmation-flow change, not an enrollment or generic form-error fix.

Screenshot/video result (before/after)

Before

The original report included missing confirmation prompting and a separate enrollment failure. The enrollment screenshot is retained as historical context only; that issue is outside the final PR scope:

Historical context - biometric enrollment failure, outside this PR's scope

https://raw.githubusercontent.com/CHUKEPC/telegram-macos/assets/pr_wallet_touchid_before.mp4

After

Original capture illustrating the explicit native confirmation prompt:

Wallet confirmation - native Touch ID prompt

https://raw.githubusercontent.com/CHUKEPC/telegram-macos/assets/pr_wallet_touchid_after.mp4

The earlier captures demonstrate the prompt's appearance, not a new enrollment result or verification of OS keychain enforcement.

Confirmation

Verification environment: macOS 27.0 (Apple Silicon), Telegram 13.1.0, build 283357 (Debug).

  • Verified the net diff contains only WalletUIIntegration.swift; shared InputDataController alert handling and biometric enrollment match master byte-for-byte.

  • Typechecked the production callbacks against Apple's LocalAuthentication framework with an x86_64-apple-macos10.13 deployment target, preserving the existing macOS 10.15 availability boundary around wallet confirmation.

  • The previously verified 31 callback-harness assertions cover the preserved per-attempt context behavior, cancelled biometrics, failed protected reads, successful authorization, invalidation, disabled biometric availability, and unchanged enrollment callbacks. These tests use production callback bodies with controlled authentication/storage responses, not hardware Touch ID or live keychain state.

  • Swift parsing and git diff --check pass. The full Debug workspace build passed with the reviewed changes integrated against upstream 70516222. The populated local build uses macOS 12.0 compatibility settings; this is distinct from the targeted 10.13 availability check, and the local settings are not included in the PR.

  • I confirm that I have tested the affected behavior and verified that it works properly, or verified the content and links for a documentation-only change.

@CHUKEPC

CHUKEPC commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Removed the global Security interposer and Mach-O pointer patching completely, including the forced login-keychain fallback. Enrollment is back on the existing PasscodeCore path with its OS-enforced .biometryCurrentSet binding and fail-closed entitlement handling. The explicit confirmation prompt remains, but authorization still requires a successful protected-key read. Updated the description and removed the claim that unsigned enrollment is a supported fix.

The full Debug workspace build passed. The retained callbacks also pass a macOS 10.13 deployment-target typecheck and 31 callback regression assertions. These are not hardware/keychain tests; the original videos are labelled accordingly.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

CONFIRMED: PR description: the title and the 'Removed'/'Restored' bullets describe the branch history, but against master the diff changes only the wallet confirmation (WalletUIIntegration.swift) and InputDataController.swift; the enrollment code (PasscodeUIHooksInstall.swift:149-166) is identical to master, so enabling Touch ID is not changed, only its failure alert becomes visible.

CONFIRMED: InputDataController.swift:894: using window instead of _window makes every .fail(.alert(...)) show on pushed and modal-hosted forms, not only wallet forms, so Business Hours now shows the hard-coded English 'Intersection Error' alert and Passport the raw "\(error)" text that were silently dropped before.

@CHUKEPC CHUKEPC changed the title Fix enabling and confirming wallet operations with Touch ID Oct 28, 2026
@CHUKEPC

CHUKEPC commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Removed the shared InputDataController alert change. The net diff now only requests fresh biometric authentication for wallet confirmation, and authorization still requires a successful OS-protected PasscodeCore key read. Enrollment and alert behavior in Wallet settings and unrelated forms match master. Updated the title and description to the confirmation-only scope. The 31-assertion callback harness, macOS 10.13-targeted availability check, and full Debug workspace build pass.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

CONFIRMED: PR description: the title and description say the PR stops reuse of an already-authorized biometric context by creating a fresh LAContext per attempt and disabling Touch ID reuse, but master already created a new, unevaluated context on every attempt and its protected key read started a fresh authentication, and touchIDAuthenticationAllowableReuseDuration = 0 (:323) only sets the documented default; the one real change is the explicit evaluatePolicy call at :324.

@CHUKEPC CHUKEPC changed the title Require fresh Touch ID authentication for wallet confirmations Oct 29, 2026
@CHUKEPC

CHUKEPC commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Corrected the title and rationale: the actual change is explicit evaluatePolicy before the protected wallet-key read. Master already creates a new LAContext per attempt, and zero reuse duration is the documented default; those are no longer described as fixes introduced by this PR. The protected-key requirement, enrollment and shared form behavior remain unchanged. This update changes only the PR wording, not the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants