Skip to content

Treat biometric cancellation as rejected authorization - #120

Open
Jim8y wants to merge 1 commit into
neoorder:masterfrom
Jim8y:fix/audit-p2-03
Open

Jim8y wants to merge 1 commit into
neoorder:masterfrom
Jim8y:fix/audit-p2-03

Conversation

@Jim8y

@Jim8y Jim8y commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Problem

Cancelling a biometric authorization could escape as an exception when the wallet was already unlocked, instead of rejecting the authorization normally. The Android native negative button also completed the prompt task as cancelled rather than returning a rejected result.

Change

  • Treat OperationCanceledException consistently as false at the existing wallet-authorization boundary, including the already-unlocked path.
  • Return false from Android's existing biometric negative button.
  • Preserve successful authorization and propagate unrelated system failures. No new authorization entry point, domain prompt, connected-app grant, or public-address permission is added.

This independent change addresses audit P2-03 only, based on master 623603d634f07eaead14745da87920a356159be0.

Validation

  • git diff --check 623603d passed during publication preparation.
  • Five focused tests are registered in tests/P2-03/AuthorizationCancellation.Tests.csproj, covering locked/unlocked cancellation, rejection, successful authorization in both wallet states, and propagation of unrelated errors. They link the production authorization service and use disposable real Neo wallets, with platform-prompt, MAUI-view and settings doubles.
  • The test project is included in OneGateApp.slnx and retains a non-build/non-output reference to OneGateApp.csproj.
  • Publication rerun: 5/5 passed, dotnet test tests/P2-03/AuthorizationCancellation.Tests.csproj --no-restore --verbosity minimal.
  • Android API 36 arm64 emulator: 3/3 native cancellation checks passed for an already-unlocked disposable wallet. The real fingerprint negative button was exercised through the production authorization service, actual ExportWalletPage, and actual SendTransactionPopup; cancellation left export data undisclosed and transfer approval incomplete. No fingerprint match, signature or transaction broadcast occurred.
  • After removing the external fixture and restoring product startup, the Android full product rebuild passed with zero warnings/errors, followed by successful install and product smoke launch. Product startup is separate from the functional cancellation evidence above.
  • iPhone 17 / iOS 26.5 simulator: 3/3 real system-biometric cancellation checks passed for an already-unlocked disposable wallet, through the production authorization service, export page and transfer popup. Simulator Face ID was enrolled; the actual system presentation was observed and cancelled. Initial unenrolled rejection and an interaction timeout were retained as failed attempts, then successfully rerun, not counted as passing evidence.
  • iOS: removed the external fixture, fully rebuilt with zero warnings/errors, installed the product and verified Home/four-tab startup. No private key was shown and no transaction was signed or broadcast.

Both platform source snapshots match the complete reviewed diff SHA256 d3be7ee2e10155ade9c8a8969edd074e74d9b0de9668a5b3edc8938bb5ef621b. Both changed production files also match the restored Android build copy byte-for-byte. Fixtures, screenshots and result JSON are not committed.

Scope and limits

This fixes the already-unlocked cancellation path. Locked-wallet cancellation and successful authorization are covered by local regression tests, not native cold-start/locked-wallet device-flow checks. iOS native rejection returns a boolean and does not demonstrate Android's prior cancellation exception. No live-wallet signing or transfer was performed.

Screenshots remain outside the repository and have not been uploaded as GitHub assets. Hosted CI results and maintainer approval are not claimed.

Copilot AI lite review requested due to automatic review settings September 5, 2026 08:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The functional changes are narrowly scoped to cancellation semantics, appear consistent with the stated contract, and are backed by targeted tests (with only a minor csproj packaging consistency nit).

Pull request overview

This PR normalizes biometric authorization cancellation behavior so user cancellation is handled as a standard “rejected authorization” (false) instead of surfacing as an exception in certain paths, and aligns Android’s negative button behavior with that contract.

Changes:

  • Catch OperationCanceledException at the WalletAuthorizationService.RequestAuthorizationAsync boundary and treat it as a rejected authorization (false) for both locked and already-unlocked wallet flows.
  • Update Android’s biometric negative button to complete authentication with false rather than canceling the task.
  • Add a focused test project for audit item P2-03 and include it in OneGateApp.slnx.
File summaries
File Description
tests/P2-03/README.md Adds run instructions and scope notes for the P2-03 cancellation test set.
tests/P2-03/PlatformStubs.cs Provides minimal platform/service stubs so the production authorization service can be exercised in tests.
tests/P2-03/AuthorizationCancellationTests.cs Adds tests covering cancellation (locked/unlocked), rejection, success, and propagation of non-cancellation failures.
tests/P2-03/AuthorizationCancellation.Tests.csproj Introduces the dedicated test project and links in the production authorization service source.
OneGateApp/Services/WalletAuthorizationService.cs Treats OperationCanceledException as a rejected authorization consistently at the service boundary.
OneGateApp/Services/DataProtectionService.Android.cs Changes biometric negative button handling to return false instead of canceling the task.
OneGateApp.slnx Includes the new P2-03 test project in the solution.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

<ProjectReference Include="../../OneGateApp/OneGateApp.csproj" ReferenceOutputAssembly="false" BuildReference="false" SkipGetTargetFrameworkProperties="true" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
<PackageReference Include="xunit" Version="2.9.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="3.1.4" />
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants