Add face verification for document issuance - #646
Conversation
Run a Regula liveness session after the NFC chip read and pass the resulting liveness transaction id to the passport issuer, which matches the live face against the chip portrait. - Add mockable RegulaFaceService interface + null-defaulting provider in yivi_core; concrete flutter_face_api implementation in yivi_app, injected via runYiviApp so the FOSS yivi_fdroid build has no Regula dependency (face verification disabled there). - Thread liveness_transaction_id into the issuance request via RawDocumentData.copyWith (withLivenessTransaction helper). - Bump vcmrtd to the branch that adds livenessTransactionId / FaceMatch. - Unit tests for the service/provider/threading and integration tests asserting the transaction id reaches the issuer.
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29250157693 |
- Add a Yivi-themed face verification intro screen shown after the successful document readout, replacing Regula's onboarding. It carries the heading, guidance and a privacy statement (processed only by Yivi on EU-sovereign servers, not stored, never shared), localized via the app's own i18n (nl/de/en). - Default to passive liveness, hide the Regula watermark, force portrait, and theme every Regula screen with the Yivi palette. - Localize Regula's remaining screens (camera hints, retry, processing, success) per the app's active language using the SDK's real string keys. - Navigate to issuance through the root navigator so issuance opens even if the NFC screen is torn down while the native liveness UI is in front. - Tests: intro screen widget tests, language forwarding, and an integration test tapping through the intro to the issuer.
- Remove the heading from the intro body (it duplicated the app bar title).
- Add a "remove facial accessories, hats, etc." guidance tip.
- Fix the Dutch privacy wording ("gezichtsafbeeldingen") and align the
English/German copy to "face images" for consistency.
|
🎬 Recorded 0 Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29253072636 |
- Add a code-drawn face-verification animation (document portrait + phone selfie, highlighted then matched with a check) on the intro screen, replacing the static icon. Follows the sibling animations' TickerMode convention so tests don't hang on the loop. - After face verification, show a loader page that leads into issuance instead of falling back to the document readout page: the screen behind the intro switches to a loader, and issuance replaces the readout route (pushReplacement) so the user is not returned to it.
- Expand the intro explanation to describe the upcoming face check and lead into the tips. - Rename the start button to "Start verification". - Hide the close button on the Regula liveness screen (the user has already confirmed on the intro screen). - Restyle Regula's camera hint labels as a light chip with dark text, to echo the intro tips.
|
🎬 Recorded 0 changed integration test(s) on iOS simulator ( Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29255111443 |
- Center-align the intro explanation and guidance tips. - Move the privacy statement into an expandable "Where does my data go?" section (reusing Collapsible) instead of an always-visible card. - Reword the privacy text to "servers hosted in the Netherlands".
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29255345586 |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29256190916 |
- Left-align the intro text and drop the collapsible; the privacy statement is shown inline as a plain hint. - Promote "point the selfie camera at your face" to a guidance tip and shorten the explanation accordingly. - Reword the preparing loader to use "gegevens" (the app's term) instead of "kaart". - Request the Yivi font (Open Sans) on the Regula screens.
Drop the "where does my data go" privacy statement from the intro screen to avoid over-informing users; remove the widget and the i18n key.
|
🎬 Recorded 0 changed integration test(s) on iOS simulator ( Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29257374477 |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29257783666 |
# Conflicts: # CHANGELOG.md # yivi_app/pubspec.yaml
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29273715635 |
flutter_face_api pins FaceSDK 8.2.4530, which is not on the CocoaPods CDN trunk (it only has up to 8.2.4516) but is hosted in Regula's own podspecs repo. Add that source (plus the default CDN, which provides FaceCoreBasic 8.2.2127) so `pod install` can resolve the Regula pods on iOS.
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Dobby consolidated review — face verification (Yivi client side)
Solid, well-tested implementation that keeps the Regula SDK out of yivi_core and the F-Droid build via the existing FOSS-injection pattern. Rule sweep (Dart list-equality, locale en/nl/de parity, bloc-transformer, tests-on-features, design-slop, WCAG) is clean except one low-severity a11y note. Requesting changes on two functional bugs below before merge.
Blocking
- 🐞 Production build defaults to the staging Face API URL (
regula_face_service.dart). - 🐞 Error path in
_startIssuanceguards on the wrongmountedand can silently swallow issuance failures (nfc_reading_screen.dart).
Non-blocking
- Lockfiles regenerated on a non-CI Flutter SDK (spurious dep downgrades).
vcmrtdpinned to an unmerged branch commit (author already flagged this).- Face-match animation lacks a
Semanticslabel (WCAG 1.1.1 A).
Rule checks that passed: Dart List/Uint8List == trap (none), locale completeness (en/nl/de all at key parity), bloc sequential-transformer (N/A — Riverpod), tests-required (service/provider/widget + integration coverage present), design AI-slop tells (none), WCAG AA copy/layout.
Tests: yivi_core unit/widget suite green locally; yivi_app integration tests are Firebase Test Lab-only (not run locally).
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29274824794 |
- Rename the screen title to Gezichtsverificatie (nl) / Gesichtsverifizierung (de); reword the accessories tip to remove glasses/hats. - Mark the intro face-match animation decorative (ExcludeSemantics) so screen readers skip it; the intro copy already conveys the flow. - Hide the Regula camera toolbar's torch/light toggle and blank the processing-screen close button (a transparent icon) so liveness cannot be cancelled mid-verification. - Skip Regula's success (checkmark) screen: the real match is server-side, so a client-side success screen would be misleading. - Show the dedicated failed_face_verification illustration on the generic error screen when the issuer rejects the face match (HTTP 400).
# Conflicts: # CHANGELOG.md
|
🎬 Recorded 0 changed integration test(s) on iOS simulator ( Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/29334903346 |
The _startIssuance catch guarded on the NfcReadingScreen's own mounted state, but the native Regula liveness UI can tear this screen down on some devices. When that happened an issuance failure was silently swallowed: no error was shown and _preparingIssuance was never reset. Fall back to the root navigator's error screen (as the success path and handlePointer do) when the screen is gone.
|
Processed the review feedback:
|
|
/dobby good job. Now review again and approve if it's good enough. We will upgrade the vcmrtd package later, so don't worry about that. |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31371137722 |
The fallback added in 5c2d171 guards the whole `_startScanning` try, but only the liveness session can tear this screen down. Everything before it (`startSessionAtPassportIssuer`, the config write, `readDocument`) runs for every issuer, including the ones that announce no face verification, and an unmounted State there means the user left the route themselves. Their failure was then pushed as a full-screen error over wherever they had navigated to, which is a behaviour change for the non-face flow. Gate the fallback on a `livenessStarted` flag set just before `withLivenessTransaction`, and replace the route instead of pushing over it, matching what `_startIssuance` already does for this same flow so dismissing the error cannot return the user to the readout page they finished. Both branches are pinned by tests, verified failing before / passing after.
382bcc5 narrowed the rejected-match check from a bare "400" anywhere in the issuer's message to the `Store failed: 400` prefix vcmrtd actually throws, but nothing tested which illustration the error screen ends up with. Cover both directions: a rejected match gets the failed-face illustration, and an unrelated failure whose body merely happens to carry those digits does not. The second case fails against the pre-narrowing check.
|
🎬 Recorded 0 changed integration test(s) on iOS simulator ( Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31375463618 |
There was a problem hiding this comment.
Rules Dobby 2, cycle 6. Verdict approve.
Re-swept at 3adf5164. The two scope nits from the last review are fixed, and I pushed the fixes to this branch rather than sending it back: they were small and specific, and the round trip has not been paying for itself on this PR.
What I changed
c6e05f39 scopes the readout error fallback to the liveness step. The fallback added in 5c2d171 guarded the whole _startScanning try, but only the liveness session can tear this screen down. startSessionAtPassportIssuer, the faceVerificationConfigProvider write and readDocument all run before any liveness UI exists, and they run for issuers that announce no face verification at all. An unmounted State there means the user left the route themselves, so their failure was being pushed as a full-screen error over wherever they had navigated to. That was a behaviour change for the non-face flow this PR otherwise does not touch. The fallback is now gated on a livenessStarted flag set just before withLivenessTransaction, and it replaces the route instead of pushing over it, which is what _startIssuance already does for this flow.
3adf5164 adds the test the Store failed: 400 narrowing in 382bcc5 never had. It covers both directions: a rejected match gets the failed-face illustration, and an unrelated failure whose body happens to carry those digits does not.
Both commits' tests were checked failing-before and passing-after by reverting only the lib/ change. yivi_core is 243/243 green, flutter analyze reports no issues, dart format is clean.
Rule sweep
Clean. I checked Dart List/Uint8List reference equality, json_serializable one-directional fields, sealed-subtype exports, lifecycle-guard promotion, the yivi_app/yivi_fdroid build-config pairing (no build config is touched by these commits), closed clientmodels enums, and the house prose rules plus the German register in the one changed locale string, which is now consistently formal Sie across that block. The only thing the sweep turned up was the missing test for the narrowed error match, which is the second commit above.
Before this merges
vcmrtd is still pinned to ref: feat/face-verification-announcement in yivi_core/pubspec.yaml:77 and yivi_app/pubspec.yaml:59. That branch still exists at 6650c960 and there is no tag above v4.0.0 carrying the announcement work, so the pin can be rebased or deleted out from under this branch. It needs the vcmrtd branch merged and tagged and both pubspecs repinned, which is the precondition you already wrote into the description. Nothing in the fix loop can do that part, so it stays with you.
Two things carried over from earlier reviews, both non-blocking: the description is stale in the three places I listed last time, and the Regula font is requested on the Dart side but not registered with the native platform.
My earlier CHANGES_REQUESTED reviews on this PR no longer describe anything open. Every finding in them is resolved, so that is stale review state rather than open work. Dismiss them when convenient and this is good to go once vcmrtd is tagged.
|
Done — I swept the 3 new fix commits against the relevant rules (all clean), then fixed the two remaining nits myself and pushed them straight to
Both were verified failing-before/passing-after, and One thing I can't fix myself: And a housekeeping note: my earlier PR: #646 |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31375687204 |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31382427688 |
|
/dobby merge master into this branch and fix merge conflicts |
|
On it — I'll merge |
Conflict in nfc_reading_screen.dart, where master's privacy-screen work and this branch's face verification both changed the NFC read call: - Imports: keep both `navigation.dart` (branch) and `privacy_screen.dart` (master). - The read: keep master's `PrivacyScreen.suspendDuring` wrapper, with the branch's `startValidation.nonceAndSessionId` inside it. The issuer's start-validation now returns a `StartValidationResult` rather than a bare `NonceAndSessionId`, so master's destructured `nonce`/`sessionId` no longer exist. Two test files needed the same reconciliation: - `nfc_reading_privacy_screen_test.dart` (new on master) stubs the issuer, so its stub returns a `StartValidationResult` with no face verification announcement — the face step stays out of a test about the NFC read. - `nfc_reading_error_surfacing_test.dart` now mocks the `privacy_screen` channel. The read runs inside `suspendDuring`, and an unmocked channel replies with a null envelope, which `MethodChannel` turns into a `MissingPluginException` — the flow then never reached the face verification or issuance errors the tests assert on.
|
Merged One conflict, in Both imports are kept: The read call is the substantive half. Master wrapped final result = await PrivacyScreen.suspendDuring(
() => _getDocumentReader().readDocument(
iosNfcMessages: _createIosNfcMessageMapper(),
activeAuthenticationParams: startValidation.nonceAndSessionId,
),
);The suspension covers only the chip read. The face verification intro and the liveness session run after it returns, outside the suspension, because those are in-app screens rather than system UI that resigns the app active. Two test files needed the same reconciliation. Neither is a behaviour change, but both are silent breakages a plain textual merge leaves behind:
Verified that the resolution dropped nothing from either side: the branch's contribution patch before the merge and after it are identical apart from the two hunks above (and the two-space reindent from the Ran locally on Flutter 3.44.9:
The blocker named in the PR description is untouched by this merge and still stands: Requested by @w-ensink. |
|
Turns out there's nothing to merge — The PR shows as blocked, but that's not a merge conflict — No commits made, nothing pushed. |
|
🎬 Recorded 0 Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31478417197 |
|
🎬 Recorded 1 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31480046024 |
|
/dobby review again and approve if it's good enough. |
|
On it — I'll re-review PR #646 and, if everything checks out against our rules, go ahead and approve it. I'll post my review shortly. |
There was a problem hiding this comment.
Rules Dobby 2 — cycle 7. Verdict approve.
Re-swept at 4ef47c62. Three commits since the last review: a master merge, set vcmrtd to v4.1.0, and fix document reading integration tests.
The dependency blocker is cleared
Cycle 1 flagged the vcmrtd pin as merge-blocking because it pointed at a branch ref (feat/face-verification-announcement), which can be rebased or deleted under us. It is now ref: v4.1.0, and that holds up:
- the tag exists and resolves to
fdc4e83a32de3199c6faeb6d1363c041eb88f926 - all three lockfiles (
yivi_core,yivi_app,yivi_fdroid) carry that exactresolved-ref - the four commits between
v4.0.0andv4.1.0are exactly vcmrtd#192, the announcement this PR reads — no unrelated work rode along, so folding the bump into this PR was the right call
The cross-repo half is merged, tagged and wired up. That was the last thing actually blocking.
What I ran
yivi_core: 253/253 passyivi_fdroid: 13/13 passflutter analyze:yivi_appandyivi_fdroidclean,yivi_corereports oneinfo • unnecessary_import(inline below, the only analyze finding in the PR)- CI at this head:
lint,unit-test,test-fdroid-build-scriptand both irmagobridge builds green. The Android/iOS app builds were still running when I looked — worth a glance before you hit merge, though nothing in the last three commits touches build config.
Rule sweep
Eight candidates, six refuted:
- Five "pending timer at teardown" flags on
nfc_reading_error_surfacing_test.dart. The scanning animations do loop on aFuture.delayedthatdispose()cannot cancel, and none of these tests do theSizedBox.shrink()+ pump-past-holdDurationteardown that usually needs. But the file passes 5/5, so the animation is not mounted on the paths these tests drive. Refuted by running it, not by reading it. FaceVerificationConfigmissing from theyivi_corebarrel. It is declared invcmrtd, notyivi_core, so any consumer importingpackage:vcmrtd/vcmrtd.dartcan already name it — which is what the integration-test helper does. Nothing in this hierarchy issealedeither.- Hardcoded
size: 20on the tip icon.IrmaThemehas no icon-size token and five other call sites inyivi_core/libalready pass a literal. House pattern, not a breach.
Locale parity holds: face_verification.* carries the same eight keys in nl, de and en, and every key the Dart side reads is defined. The changelog entry sits under ### Added and both entry points really do inject a concrete service, so it is not describing dead code. The yivi_app / yivi_fdroid dependency divergence is the deliberate FOSS split, and the version: lines still match.
What is left
Five non-blocking items inline: two style, two nits, one a11y note. None of them should hold the merge, and the two pubspec ones are single-line if you want them.
The on-device list in your own PR body is still the real remaining risk and not something I can check from here — the Regula screens' look, the hidden close/torch buttons, the skipped success screen across SDK versions, the WebView capture page on a real F-Droid build, and the Regula font that still needs registering with the native platform.
|
Reviewed and approved ✅ — review 4906109948, with 6 non-blocking inline comments. The thing that was actually risky checked out fine: Left inline, none blocking: the repin dropped some useful pubspec comments from #695 (why we avoid The on-device checks called out in the PR body itself (Regula screen look, hidden close/torch buttons, F-Droid capture page, native font registration) are still worth a manual pass since I can't verify those from here. |
|
🎬 Recorded 3 changed integration test(s) on iOS simulator (
Download the videos from the Artifacts section of the run: https://github.com/privacybydesign/irmamobile/actions/runs/31488302550 |
3 participants
Implements the Yivi client side of face verification per the go-passport-issuer face-verification design.
After the NFC chip read, the app runs a liveness session (camera) and passes the resulting
liveness_transaction_idto the passport issuer, which matches the live face against the document chip portrait (DG2/DG6) server-side and gates issuance on the result. Applies to passport, ID card and driving licence.Flow
The passport issuer decides per session whether face verification applies: its start-validation response carries a face verification announcement, and the announcement's presence is the signal. Without one the app skips the step entirely and goes straight to issuance, so the feature can be switched off remotely and old issuers keep working.
When it does apply: document readout → Yivi face-verification intro screen → liveness session (passive) → "preparing" loader → credential issuance.
On a matching face, issuance proceeds directly. When the issuer rejects the face match (HTTP 400), the app surfaces the generic issuance error screen (retry / cancel) with a dedicated failed-face illustration; retry re-runs the readout and liveness. A liveness session that produces no transaction id is failed in the app rather than sent on: the issuer announced the step, so a request without the id would leave the face check silently missing.
Architecture
Follows the repo's existing FOSS injection pattern (as used for OCR / QR), so the proprietary Regula SDK stays out of
yivi_coreand the F-Droid build:yivi_core— mockableRegulaFaceServiceinterface,RegulaLivenessResult, and aregulaFaceServiceProviderdefaulting tonull(disabled).withLivenessTransaction()attaches the transaction id to the issuance request viaRawDocumentData.copyWith.faceVerificationConfigProviderholds the issuer's announcement for the current flow, which also names the Face API the session must target.yivi_app— concreteRegulaFaceServiceImplbacked byflutter_face_apiin web-service mode (the Regula license lives on the Face API server; no license file ships). Injected viarunYiviApp(regulaFaceService: ...), built against the announced Face API URL, so no environment is pinned at compile time.yivi_fdroid—RegulaWebFaceService: the same liveness step through Regula's web Face SDK, running on a Yivi-hosted capture page that the issuer serves under/captureand that the app loads in an embeddedwebview_flutterWebView (F-Droid face verification via Regula's web Face SDK #665/feat(fdroid): FOSS liveness via Regula web Face SDK in a WebView #666). The FOSS build therefore has face verification with no proprietary native code in the APK: it ships the BSD WebView plugin and the page URL, and Regula's code executes remotely. Cancel, back and page errors throw, so the flow lands on the same issuance error screen as the native build.Client UI
ExcludeSemantics) so screen readers rely on the intro copy. Copy is localized through the app's own i18n (nl/de/en); the screen is titled "Gezichtsverificatie" / "Gesichtsverifizierung" / "Face verification".Scaffold/app bar, so there is no browser chrome and the WebView is granted the camera and nothing else.Dependencies
vcmrtd(yivi_core + yivi_app) is pinned toref: feat/face-verification-announcement(+path: vcmrtd, since the package lives in a subdirectory): v4.0.0 plus the commit that adds the issuer's face verification announcement toStartValidationResult, alongsideRawDocumentData.livenessTransactionId/copyWith. This branch has to be merged and tagged in vcmrtd, and both pubspecs repinned to that tag, before this PR merges: a branch ref can be rebased or deleted under us.flutter_face_api+flutter_face_core_basictoyivi_apponly, andwebview_flutter+webview_flutter_androidtoyivi_fdroidonly.Tests
yivi_core: unit tests for the service/provider/threading and language forwarding, including that a session without a transaction id fails; unit tests for the capture URL the FOSS flow derives from the issuer; widget tests for the intro screen and for where a readout failure is surfaced when the screen was torn down.yivi_fdroid: unit tests for the capture-page message parsing and forRegulaWebFaceService.yivi_app: integration test taps through the intro and asserts the transaction id + active language reach the issuer, and that no id is sent when the issuer announces nothing.Notes for reviewers / follow-ups
Store failed: 400during a face-verification issuance.res/font/assets; iOSUIAppFonts+ bundled ttf) for it to actually render. Not yet wired.Cache-Control, so the WebView clears its cache before each load; the durable fix isCache-Control: no-storeon the served page.