Commit graph

3 commits

Author SHA1 Message Date
cf878d9e3e feat(pay): coordinator entry point for an externally granted authorization
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
The API change tranche 10 scoped, in REVIEWS/adr/0007.

## dispatch_with_authorization

Tranche 10 established that Android cannot implement BiometricAuthorizer
without reopening defect S2, and built AuthorizationAttempt as the
replacement. It named the remaining work: a coordinator entry point taking
an already-granted attempt instead of calling biometric.authenticate().

  coordinator.dispatch_with_authorization(&id, &mut attempt)

It does not prompt, does not wait, and does not consult the injected
BiometricAuthorizer at all. That is asserted rather than assumed: one test
injects an authorizer reporting no hardware that also errors, and shows a
granted attempt still dispatches. If the coordinator ever fell back to it,
that test fails.

Three refusals, each with a test that fails when the gate is removed:

- An unanswered prompt does not dispatch. PromptShown plus SensorEngaged
  is not consent — the exact shape the Android adapter would produce.
- A grant belongs to one payment. A foreign grant is refused before
  anything else, so a stray callback cannot even fail the payment on
  screen; the victim intent stays in AwaitingUserAuthorization rather
  than being transitioned to Failed by a stranger.
- A grant is spent on use. One authorization, one dispatch (B3).

Denied, still-prompting and already-spent attempts all fail the intent
closed and never reach the gateway.

## Verifying the tests can fail

The gate was removed (if !attempt.consume() -> if false) and the suite
re-run: 5 tests failed. A fail-closed test that passes against fail-open
code is worthless, so this is the check that matters.

Worth recording: a_grant_cannot_dispatch_twice still passed with the gate
removed, because the existing duplicate-dispatch budget caught it
independently. Two unrelated mechanisms refuse the second dispatch. That
is defence in depth working, and the reason that test is not sufficient
evidence on its own.

## Validation

  domain  : 137 tests --locked, fmt, clippy -D warnings, bench    pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy               pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard        pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check      pass
  authorization / batch / settlement-tick guards                  pass
  fail-open injection: 5 tests fail with the gate removed         pass

Domain tests 129 -> 137.

## What remains

The authorization half is done and verified. PayFlowHandler still owns the
USSD session lifecycle, the pending-store writes that shadow the
coordinator's repository, and the bulk queue plumbing. Moving those makes
the coordinator own the gateway session, which changes who cancels on
teardown and who observes an out-of-order callback — ADR 0007's device
matrix, which cannot be exercised here.
2026-07-29 04:10:59 +00:00
cbafa92269 fix(pay): close the fail-open biometric the coordinator migration would open
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Review defect S2, plus an amendment to ADR 0007.

## Attempting the migration found a defect in the plan

The stated next step was replacing PayFlowHandler with PaymentCoordinator,
which means writing a BiometricAuthorizer adapter for Android. It cannot
be written correctly, and why is the substance of this change.

The trait contract is blocking: "authenticate must be a blocking call that
waits for user action. Returns Ok(()) on success."

robius_fingerprinting::authenticate does not do that. Reading through
sys/android/prompt.rs: it calls the Java authenticate static method and
returns Ok(()) as soon as the prompt is on screen. The user's answer
arrives later through next_event().

So an adapter can either return Ok(()) when the prompt opens — and
authorize_and_dispatch then sends money before the user has touched the
sensor, which is defect S2 reintroduced through the type system — or
block, and deadlock the thread that must pump the callback.

Had the migration been done without noticing, the result would have been a
correct-looking refactor that silently reopened the review's most serious
security finding.

## AuthorizationAttempt

Authorization is a state machine, not a function call:
Requested -> Prompting -> Granted | Denied, advanced by inbound signals.
One rule, enforced by the type: only an explicit success grants dispatch.

- A displayed prompt does not authorise. A touched sensor does not
  authorise. A non-match keeps the prompt up and stays retryable.
- A grant is bound to one intent, so a callback for an abandoned payment
  cannot authorise the current one.
- A grant is spent on use, so one fingerprint cannot authorise two
  dispatches (B3), and a replayed success cannot re-arm it.
- A late success after a cancel is ignored, not resurrecting the payment.
- Backgrounding mid-prompt denies; it never silently allows.

dispatch_ussd now consumes a grant before dispatching and refuses without
one. auth == None means no biometric gate was configured, which is
deliberately distinct from an ungranted one. Both cancel paths abandon the
grant.

A CI guard asserts the gate exists and was tested with the check disabled
to confirm it fails.

The trait keeps its blocking contract for synchronous authorizers and test
doubles, and now documents that Android must not use it.

## Validation

  domain  : 129 tests --locked, fmt, clippy -D warnings, bench    pass
  storage : 36 + 41 sqlcipher --locked, fmt, clippy               pass
  platform: 56 + 64 ussd --locked, fmt, clippy, mock guard        pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check      pass
  authorization guard: verified to fail with the gate removed     pass
  batch-counter and settlement-tick guards: still passing         pass

Domain tests 117 -> 129.

## Where the migration stands

The blocker is no longer unknown. PaymentCoordinator needs an
authorization path that does not assume a blocking authorizer, and
AuthorizationAttempt is that path, built and tested. What remains is a
coordinator entry point taking an already-granted attempt instead of
calling biometric.authenticate() itself, then moving USSD session
ownership across.

That is an API change that should be designed against ADR 0007's device
matrix — permission denial, cancellation, backgrounding, app restart,
out-of-order callbacks — none of which can be exercised here.
2026-07-29 03:57:07 +00:00
d0f5e74435 feat(pay): Phase 5 platform gateway boundary, and verify Phase 4
Some checks failed
nigig-map / test (push) Has been cancelled
Payment domain, storage and platform / isolated-payment-tests (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
Phase 5 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. Design and the one
item engineering cannot close are in REVIEWS/adr/0007.

## Phase 4 is now verified, not just written

Tranche 5 implemented the draw_walk fixes and said plainly that the
nigig-pay widget edits were unbuilt. That blocker was environmental:
installing the packages tools/makepad-native-libs.sh already lists makes
the UI graph compile. Both commands the status doc listed as required
now pass:

  cargo check -p nigig-pay      pass
  cargo check -p nigig-pay-ui   pass

No code changed for this; the claim is now evidence rather than assertion.

## Phase 5: new nigig-pay-platform crate

ADR 0002 reserved this crate and marked it "not yet created".

5.1 One crate owns the seam. It is the only crate in the payment stack
that may name a platform SDK. CI enforces both directions: payment crates
may not import Makepad, and domain/storage may not import jni or the
robius platform crates.

5.3 Correlation is mandatory and single-flight. SessionRegistry admits an
event as Accepted, Duplicate or Ignored; PlatformEvent cannot be built
without a CorrelationId. Closed session ids are retired permanently, so
an abandoned session's confirmation cannot settle the payment that
replaced it. There is a test named after exactly that scenario.

2.8/5.3 Progress decides retry safety, not error kind. classify_failure
takes the failure and the DispatchProgress reached before it, and
progress is the authority. The same TemporarilyUnavailable is safely
retryable before the dial and ambiguous once the menu is being driven —
the distinction the old code could not make, which is defect B3's
mechanism. A property test asserts across the whole failure space that
nothing which may have reached the provider authorises a fresh attempt.

5.4 Fakes cannot ship. MockGateway is cfg-gated, is a compile_error! in a
release build unless allow-mock-in-release is named explicitly, and
stamps every session id with MOCK-. CI asserts the release build fails.

5.5 No unsafe, no PIN. The crate is #![forbid(unsafe_code)] so the JNI
surface stays in robius-ussd. UssdGateway is !Send/!Sync by construction,
making the main-thread requirement a compile error. The adapter leaves
the pin field empty and a test asserts it.

5.6 The web claim is withdrawn. No browser API can drive USSD and a
Daraja credential must never reach a browser, so WebGateway refuses every
call and maps to Fatal — "never sent" — which owes no reconciliation.

7.5 Adversarial SMS corpus. StrictMpesaSms is the payment-boundary
reader, deliberately separate from nigig-core's permissive tracker parser
(ADR 0007 explains why this is not the duplication ADR 0002 forbids). It
requires an exact 10-char code, exact sender-ID match so MPESA-REFUNDS
and FAKE-MPESA are refused, rejects fractional shillings instead of
rounding, and caps body length. Corpus covers spoofing, forged code
shapes, out-of-range amounts, unicode and NUL injection, and replay. The
closing test asserts the honest limit: a well-crafted forgery is still
only evidence, because the output type has no settled state to reach.

## 5.2 is not done and is not closeable here

The AccessibilityService Play-policy review is a business decision. ADR
0007 records it as blocking, states the termination exposure, and names
what must happen before the rail is enabled. USSD dispatch stays behind
the default-off demo feature. If the review fails, ADR 0001's
tracker/launcher position applies and only the dispatch adapter is lost.

## Validation

  platform: 56 tests, 64 with --features ussd, fmt, clippy -D warnings
            (both feature sets), cargo-deny, mock-in-release guard
            asserted to fail                                      pass
  domain:   75 tests --locked                                     pass
  storage:  36 + 41 tests --locked, incl. sqlcipher                pass
  nigig-pay, nigig-pay-ui: cargo check                            pass

cargo-deny reports advisories/bans/licenses/sources ok. The isolated
runner gained a `platform` target and it runs in CI on every push that
touches the crate.
2026-07-29 02:07:19 +00:00