SDKS-4613 Create simple journey sample app for flutter using new Ping Orchestration SDKs - #124
SDKS-4613 Create simple journey sample app for flutter using new Ping Orchestration SDKs#124rodrigoareis wants to merge 1 commit into
Conversation
… Orchestration SDKs
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Really nice piece of work 👏
- The core architecture is the right call. Keeping live
Journey/ContinueNodeobjects native and passing only opaque UUID handles across the channel is exactly how a Pigeon bridge over a stateful SDK should be built. That decision is what makes the rest of the design tractable. - Pigeon codegen is consistent at v27.1.2 across Dart, Kotlin and Swift — no stale-generation drift anywhere in 210 files, which is easy to get wrong and annoying to debug later.
- The test suite is well above what sample apps usually ship.
node_mapper_test.dart(~20 cases), a complete fake HostApi injourney_client_test.dart, widget tests for the journey/success/config views plus five callback views, and on the native side real SDK types with negative paths.JourneyNodeMapperTests.swift:109-119asserting thatPasswordCallbacknever leaks its value is a security-minded test somebody chose to write.JourneyNodeMapperTests.swift:51-61deliberately reaches an otherwise-unreachabledefaultbranch and documents why it's reachable on iOS but not Android — that's real care. - The genuinely subtle cross-platform traps were spotted and handled.
TextOutputCallback._parseMessageTypenormalising Android's"INFORMATION"against iOS's"information"; the ms→s timeout conversion inJourneyConfigParser.swift:34being correct because iOSWorkflowConfig.timeoutis aTimeIntervalin seconds while Android's isLongmillis; theErrorNode.statusasymmetry being explicitly documented rather than silently papered over. Those are the details that separate a bridge that works from one that only works on the platform you tested. - Housekeeping is clean too: CHANGELOGs in every package,
public_member_api_docs: trueenforced on the bridge with the public API actually documented, and no secrets, keystores orlocal.propertiescommitted.
The findings below are almost all in the areas the tests don't reach yet — JourneyHostApiImpl and JourneyConfigParser on both platforms, plus the build/packaging files. That's not a coincidence, and it's the most useful thing I can tell you: the code that got tested is in good shape.
What I'd like to see before merge
Grouping the blocking ones so the shape is clear:
1. The release and packaging paths appear never to have been exercised. ping_journey.podspec declares only s.dependency 'Flutter' while its sources import PingJourney, PingOrchestrate, PingOidc, PingJourneyPlugin and ping_core — any CocoaPods host app fails to compile. Both Android build.gradle.kts files have plugins {} after group/buildscript/allprojects, which Gradle's Kotlin DSL rejects outright. The release manifest has no INTERNET permission. And Class.simpleName as the callback wire key breaks under R8 in release while working perfectly in debug.
2. Sign-off doesn't reliably end the session — and returns true regardless. Details inline; I checked this against the pinned journey-2.0.0 sources rather than inferring it.
3. Concurrent next() isn't serialised on either platform, so a double-tapped Submit is a real failure mode (worse on iOS, where it's a data race rather than just a lost update).
4. Two changes look unrelated to SDKS-4613 — the hardcoded tenant in the SwiftUI sample and the ping-ios-sdk 2.1.0 bump. Happy to be told these were deliberate.
A CI job for the Flutter tree would have caught a good number of these automatically — .github/workflows/CI.yml is currently scoped to paths: ['javascript/**/*'], so nothing in this PR was verified by CI. Worth landing alongside.
I've marked the non-blocking items clearly as [optional] — please feel free to skip, defer or disagree with any of those; they're observations, not requests. Happy to talk through any of the blocking ones if you see it differently, and glad to re-review quickly.
Note on my own verification: I could not run flutter analyze, flutter test, Gradle or Xcode — no Flutter/Android/Xcode toolchain available to me here. Everything below is from reading the PR head (888fa92) against merge base aff3c6c, cross-checked against the pinned SDK sources (journey-2.0.0-sources.jar, ping-ios-sdk 2.0.0). The Gradle plugins {} ordering and podspec compile failures are inferred from the build files rather than observed from a failed build, so please sanity-check those two against a real build.
| s.author = { 'Ping Identity' => 'oss@pingidentity.com' } | ||
| s.source = { :path => '.' } | ||
| s.source_files = 'ping_journey/Sources/ping_journey/**/*' | ||
| s.dependency 'Flutter' |
There was a problem hiding this comment.
Blocking — the podspec can't compile: none of the native dependencies are declared.
source_files globs in sources that import PingJourney (JourneyHostApiImpl.swift:9), PingOrchestrate (:10), ping_core (:11), PingJourneyPlugin (Mapper/JourneyNodeMapper.swift) and PingOidc (JourneyConfigParser.swift) — but the only declared dependency is Flutter.
Any host app on CocoaPods fails at compile with error: no such module 'PingJourney'. That's still the default flutter create output, and README.md:82 explicitly offers CocoaPods as "an optional fallback for host apps not yet on SPM". pod lib lint (which line 3 tells the reader to run) would surface this immediately.
This also makes README.md:75-77 inaccurate — "Both platforms pin the native Ping SDKs to 2.0.0" holds for the SPM path (Package.swift:17, exact: "2.0.0") but the CocoaPods path pins nothing, because it declares nothing.
Two smaller things on the same file: s.license = { :file => '../LICENSE' } (line 14) resolves to ping_journey/LICENSE, which doesn't exist — the LICENSE is at flutter-sdk-bridge/LICENSE, one level up (same bug in ping_core.podspec:14). And s.swift_version = '5.0' (line 23) contradicts Package.swift's swift-tools-version: 5.9.
| } | ||
| } | ||
|
|
||
| plugins { |
There was a problem hiding this comment.
Blocking — plugins {} is in a position Gradle rejects, and the Kotlin plugin is never applied.
This block sits after group/version (lines 1-2), buildscript {} (4-15) and allprojects {} (17-22). In the Kotlin DSL only buildscript {} and pluginManagement {} may precede plugins {}; anything else fails the script with "all plugins blocks must appear before any other statements in the script file". Same in ping_core/android/build.gradle.kts:24.
The Flutter Kotlin-DSL plugin template uses apply(plugin = "com.android.library") / apply(plugin = "org.jetbrains.kotlin.android") specifically to avoid this — this reads like a refactor of that template where plugins {} was swapped in but not moved to the top.
The same edit also dropped the Kotlin Android plugin: kotlin-gradle-plugin:2.3.20 is on the buildscript classpath (line 13) but never applied, which leaves kotlin { compilerOptions { … } } (68-72) without a receiver and the versionless testImplementation("org.jetbrains.kotlin:kotlin-test") (79) without the alignment that normally resolves it. AGP 9's built-in Kotlin support may cover part of that — but the ordering error is unconditional.
I couldn't run Gradle to confirm, so worth a quick ./gradlew :ping_journey:assembleRelease to see the actual message.
| @@ -0,0 +1,45 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android"> | |||
There was a problem hiding this comment.
Blocking — no INTERNET permission, so release builds have no network access.
android.permission.INTERNET appears only in src/debug/AndroidManifest.xml and src/profile/AndroidManifest.xml. That's the Flutter template's debug-only injection for hot reload — it is deliberately not merged into release.
So flutter build apk --release produces an authentication sample that cannot reach the network: every journey request fails, and because debug worked perfectly the developer has no signal pointing at the manifest. Needs an explicit <uses-permission android:name="android.permission.INTERNET" /> here.
| */ | ||
| internal object JourneyCallbackValueApplier { | ||
| fun apply(node: ContinueNode, values: List<CallbackValueMessage>) { | ||
| val callbacksByType = node.callbacks.groupBy { it::class.java.simpleName } |
There was a problem hiding this comment.
Blocking — Class.simpleName as the wire key breaks under R8, release-only.
This groupBy { it::class.java.simpleName } and the matching JourneyNodeMapper.kt:68 (callback::class.java.simpleName) make the entire {type, index} protocol depend on class names surviving minification. They don't: with isMinifyEnabled = true (very common for release), NameCallback becomes something like a.b.c.
Failure chain: the mapper emits CallbackMessage(type = "c") → Dart's node_mapper.dart switch has no case for it → falls to _ => → the login screen renders the literal text Unsupported callback: c where the username and password fields should be. Submitting is equally broken, since this groupBy can never match "NameCallback" again. Works flawlessly in debug, completely broken in release.
Worth noting neither bridge module declares consumerProguardFiles, and none of the four Ping AARs ships a proguard.txt, so nothing protects these names today.
The native SDK avoids this entirely — CallbackRegistry.callback() keys off the server-sent "type" string. AbstractCallback.json already carries that value, so a stable key is available without a protocol change.
| scope.launch { | ||
| val result = runCatching { | ||
| val handle = resolveHandle(journeyId) | ||
| if (handle.hasOidc) { |
There was a problem hiding this comment.
Blocking — signOff can leave the session fully intact and still report success.
I verified this against the pinned journey-2.0.0 sources rather than inferring it, because the consequence is significant.
The only thing that actually terminates a Journey session is Workflow.signOff() — the SDK's Session module hooks it to POST …/sessions?_action=logout and call tokenStorage.delete(). This bridge never calls it directly; it's reached only via UserDelegate.logout(), which needs both hasOidc == true and journey.user() != null. And Journey.user() returns null when session() is null (User.kt:40-42).
Scenario — session-only journey (serverUrl, no OIDC; a supported config, per hasOidcConfig): user logs in, taps Sign out. handle.hasOidc is false, so this if body is skipped entirely. Only local node caches are cleared, and line 146 returns true. Dart shows the logged-out screen, but the SSO cookie is still in EncryptedDataStoreStorage and the AM session is still valid server-side. On the next start() the Session module replays the stored cookie and the user is silently re-authenticated with no credentials. On a shared or kiosk device, the next person gets the previous user's session.
Second scenario, OIDC path: if the SSO cookie expired but tokens are still stored, user() returns null, logout() never runs, tokens are never revoked or deleted — and this still returns true.
Separately, UserDelegate.logout() swallows Workflow.signOff()'s Result.failure, so even the happy path returns true when server-side logout failed. As written the Boolean return is a constant, which makes if (await signOff()) on the Dart side misleading.
JourneyHostApiImpl.swift:110-127 has the same structure, so this needs fixing on both sides.
| val map = asMap(value) | ||
| (map["selectedQuestion"] as? String)?.let { callback.selectedQuestion = it } | ||
| (map["selectedAnswer"] as? String)?.let { callback.selectedAnswer = it } | ||
| (map["allowUserDefinedQuestions"] as? Boolean)?.let { |
There was a problem hiding this comment.
[optional] allowUserDefinedQuestions is a server-authoritative flag being set from client input.
This is a policy flag the server declares; letting Dart write it back through the bridge inverts that. Low practical impact since the server re-validates, but it's the kind of thing worth not modelling in a reference bridge.
Separately, applyKba (lines 66-73) silently no-ops on bad input — (map["selectedAnswer"] as? String)?.let { … } means a wrong type or missing key applies nothing and next() proceeds with an empty answer, which the server rejects for reasons the developer can't see. Every sibling helper (asString/asBoolean/asInt/asDouble/asMap, lines 75-94) throws IllegalArgumentException on mismatch instead. The test at JourneyCallbackValueApplierTest.kt:137-149 currently pins the silent behaviour as intended, so this may well be deliberate — flagging in case it isn't.
For comparison, JourneyCallbackValueApplier.swift bounds-checks with guard value.index >= 0, Int(value.index) < matching.count where Kotlin relies on getOrNull(…) ?: throw. Same outcome, but the error text reaching Dart differs.
|
|
||
| /// Maps a [CallbackMessage] to the matching [Callback] subtype, keyed by its native class-name | ||
| /// [CallbackMessage.type]. Unrecognized types are not representable in the v1 sealed hierarchy | ||
| /// and are dropped by the caller (see [ContinueNode.callbacks] construction above, via |
There was a problem hiding this comment.
[optional] This doc comment contradicts the implementation.
It says unrecognized types "are dropped by the caller", but line 154's _ => renders them: TextOutputCallback(… message: … ?? 'Unsupported callback: ${message.type}').
The risk is an integrator trusting the doc, assuming unknown callbacks are invisible, and shipping a UI that prints Unsupported callback: XyzCallback to end users. Since the fallback is arguably the better behaviour, updating the comment to describe it seems like the right fix.
| path: '/journey-name', | ||
| builder: (context, state) => JourneyNameView( | ||
| onSubmit: (name) { | ||
| context.read<JourneyViewModel>().startJourney(name); |
There was a problem hiding this comment.
[optional] startJourney is fired without await before navigating.
startJourney calls await dispose() then start(), but navigation happens immediately on line 31, so JourneyView builds against whatever node the view model still holds. On a re-entry that's the previous journey's node, briefly rendering a stale form. And if startJourney throws, the resulting _error is never displayed on the screen the user just landed on (see my note on journey_view_model.dart).
Line 48 also reaches for the global router where the surrounding code uses context.go — worth making consistent.
Two more small ones in the same area: journey_view.dart:48,58-64 navigates from build via addPostFrameCallback guarded by _successHandled. It works, and there's a test covering the exactly-once behaviour — but an explicit view-model event or stream would be a sturdier pattern to demonstrate in a sample. And journey_view_model.dart's signOff() never clears _node, so post-logout state still references the completed journey.
| @@ -0,0 +1,10 @@ | |||
| name: flutter_workspace | |||
There was a problem hiding this comment.
[optional] Root workspace declares no dependencies, but analysis_options.yaml needs one.
flutter/analysis_options.yaml is include: package:flutter_lints/flutter.yaml, and this file has no dependencies or dev_dependencies, so flutter_lints can't be resolved at the root — root-level analysis either errors or silently applies no lints. A dev_dependencies: flutter_lints: entry here should sort it.
A couple of other packaging odds and ends, all optional:
PrivacyInfo.xcprivacyexists in bothping_coreandping_journeybut is commented out inPackage.swift:28/:32and in both podspecs (:29). So it ships as neither a processed SPM resource nor a pod resource bundle — under CocoaPods the**/*glob sweeps it intosource_filesinstead. The plugin transitively pulls inPingStorage(Keychain) andPingDeviceId, so App Store Connect's required-reason-API validation has nothing to read. Either uncomment both lines or delete the files.- Both
AndroidManifest.xmlfiles in the bridge still declare thepackage="…"attribute alongsidenamespacein Gradle. AGP removed support for that attribute; against AGP 9.0.1 it's at minimum a hard warning. Current Flutter templates omit it. flutter-journey/android/app/build.gradle.kts:30-32signs release with the debug keystore (template default, and the TODOs are still there), andios/Runner/Info.plisthas noCFBundleURLTypes— README step 5 asks the user to add it by hand, but without it thefrauth://redirect never returns to the app and the sample dead-ends after authentication. Pre-wiring that would save every first-time reader the same debugging session.ios/RunnerTests/RunnerTests.swiftis still the empty templatetestExample(), andping_corehas no native tests on either platform (ping_core/android/src/contains onlymain/, thoughbuild.gradle.kts:42-44configures asrc/test/kotlinsource set) — which means the registry at the centre of the leak finding has no test anywhere.
| @@ -0,0 +1,165 @@ | |||
| // Copyright (c) 2026 Ping Identity Corporation. All rights reserved. | |||
There was a problem hiding this comment.
[optional] Some dead protocol surface, and a few dangling doc references.
NodeMessage.input and CallbackMessage.raw are declared on the wire and consumed by neither platform (raw in particular would solve the unmapped-callback dead-end — see my note on JourneyNodeMapper.swift), and validateOnly is documented as read-only with no return path. Unused fields in a generated-code contract get expensive to remove once anyone depends on the schema, so now is the cheap moment to either fill or drop them.
Unrelated but same category — these paths don't exist in the repo:
DESIGN.md, referenced inPingCorePlugin.swift:13,PingCorePlugin.kt:15,ping_core/lib/src/session.dart:12andflutter-journey/lib/routing/router.dart:17AGENT_NOTES.md, referenced in both integration tests
Each one sends a reader looking for a file that isn't there. If DESIGN.md exists somewhere internal it's worth either committing it or dropping the pointers; AGENT_NOTES.md looks like a working-notes file that didn't make the commit.
Two more, both minor:
JourneyHandle's doc comment (JourneyClientFactory.swift:14-17) saysJourney.journeyUser()"never returns nil" — it returns nil when the shared context has no OIDC config or whensession()is nil (User.kt:40-42). ThehasOidcflag is still worth keeping, but a wrong justification on a defensive flag invites someone to delete it. The Kotlin comment (JourneyClientFactory.kt:18-21) is accurate for Android.messageTypeon the wire is whatever each platform's reflection prints (String(describing:)vs.name). It works because Dart normalises case, butString(describing:)on an enum isn't a stability guarantee — it'll shift the moment either SDK renames a case. Normalising at the native boundary would make the contract explicit.kba_create_callback_view.dart:13usesconst _customQuestion = '__custom__'as aDropdownMenuItemsentinel; a predefined question with that exact text would collide. Unlikely, but a nullable value or a wrapper type would rule it out.
|
|
||
| plugins { | ||
| id("dev.flutter.flutter-plugin-loader") version "1.0.0" | ||
| id("com.android.application") version "9.0.1" apply false |
There was a problem hiding this comment.
The Android project uses AGP 9.0.1, but Flutter 3.44 still relies on the legacy Android Gradle Plugin DSL. Android Studio failed for me during Gradle sync with an ApplicationExtensionImpl to AbstractAppExtension cast error.
To preserve compatibility with Flutter 3.44, I added gradle.properties with the following, and gradle sync succeeded...:
android.newDsl=false
android.builtInKotlin=false
There was a problem hiding this comment.
I had an issue opening the Runner.xcworkspace - it was opening blank in Xcode...
It turned out that the root .gitignore excludes *.xcworkspacedata globally. Claude added the following in this .gitignore file and Runner.xcworkspace in Xcode worked correctly.
!Runner.xcworkspace/contents.xcworkspacedata
3 participants
Summary
Adds a Flutter sample app demonstrating authentication against a PingAM/PingOne Advanced Identity Cloud Journey, built on a new native-wrapper bridge plugin that exposes the published native Android/iOS Ping Orchestration SDKs to Flutter. This bridge — not a pure-Dart reimplementation — wraps the existing native SDKs via
Pigeon-generated, type-safe Dart ⇄ Kotlin ⇄ Swift bindings.
ping_core— shared, process-wide native registry (CoreRuntime) for live SDK handles(
Journey,ContinueNode, ...) that never cross the platform channel, plus small sharedhelpers (
PingException, JSON codec). Exposes no PigeonHostApiof its own.ping_journey— the Pigeon schema, generated Dart/Kotlin/Swift code, and native Journeyorchestration logic (
JourneyClientFactory,JourneyConfigParser,JourneyHostApiImpl,node/callback mappers, error mapper). Depends on
ping_coreand the native Journey SDK(Android
com.pingidentity.sdks:journey, iOS SPMPingJourney, both pinned to2.0.0).Username/Password, Choice, KBA, Terms and Conditions, Text Input/Output, and
String/Number/Boolean Attribute Input).
next()and recovers from anErrorNode/FailureNodewith a"Try Again" button.
flutter/pubspec.yamlsoflutter pub getfromflutter/resolves
flutter-sdk-bridge/ping_core,flutter-sdk-bridge/ping_journey, andflutter-journeytogether.Test plan
flutter pub getfromflutter/resolves the workspace cleanlyflutter testpasses forping_core,ping_journey, andflutter-journey(unit + widget tests)
flutter runfromflutter-journey/against a configured PingAM/AIC tenant:oidcConfigis setintegration_test/journey_login_test.dartandjourney_registration_test.dartpassagainst a booted simulator/emulator or device (run standalone, not combined)
flutter build)