feat(android): add kotlin-pingonemfa sample app - #129
Open
EvgeniyMish wants to merge 3 commits into
Open
Conversation
New Android Kotlin/Compose sample demonstrating PingOne MFA SDK integration: QR-code account pairing, TOTP display with live countdown, and push-notification approve/deny flows (default, number-challenge, dry-run, and server-cancel). Key implementation details: - PingOneSampleApplication performs one-time SDK init and FCM token registration in a process-lifetime CoroutineScope, sequentially (token registration runs only after initialize() succeeds) so the device is always registered for push on first install. - PushNotificationService forks foreground vs. background on arrival: foreground launches PushNotificationActivity directly via startActivity (BAL-safe because ProcessLifecycleOwner confirms foreground); background posts a high-priority notification with setFullScreenIntent so the OS wakes the screen from lock without the service calling startActivity. - PushNotification is carried as a Parcelable Intent extra so PushNotificationActivity survives process death between banner post and user tap. PushNotificationStore is narrowed to in-process cancel-path coordination only. - NotificationCancelBus uses MutableSharedFlow(replay=1) so a cancel signal emitted before PushNotificationActivity subscribes is not dropped. - PushNotificationViewModel owns approve/deny in viewModelScope so in-flight SDK calls survive Activity recreation. onNewIntent swaps the displayed notification via Compose state (no recreate()) and calls resetState() to clear stale dialog results atomically. - OTP countdown uses a SystemClock.elapsedRealtime() deadline anchored at SDK response time and stored in ViewModel, so the counter resumes at the true remaining time after navigation rather than resetting to 30 s. otpVersion counter guarantees LaunchedEffect restarts on every generateOtp() outcome including failure. - QrCodeAnalyzer is a single remembered instance with DisposableEffect cleanup; scanned guard uses AtomicBoolean for thread-safe compare-and-set between camera executor and composition threads. CameraProvider binding uses addListener(ContextCompat.getMainExecutor) to avoid blocking the main thread and satisfy CameraX @mainthread assertions. - All strings externalised to strings.xml; MIT copyright headers on every source file; DiagnosticLogger StateFlow updates use update() for atomicity.
|
Warning Review limit reached
Next review available in: 17 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a Kotlin Android PingOne MFA sample. It now initializes the SDK, loads accounts, generates OTPs, supports QR and manual pairing, handles push approvals and denials, and shows diagnostic logs in Compose screens. ChangesPingOne MFA Android sample
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/OtpBox.kt (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the KDoc reference to the icon.
[Tag]does not resolve to an imported type. The import isandroidx.compose.material.icons.filled.Tag, which is a property onIcons.Filled. Write the reference as plain text or asIcons.Default.Tag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/OtpBox.kt` at line 42, Update the KDoc description in OtpBox to replace the unresolved [Tag] reference with plain text or a valid Icons.Default.Tag reference, matching the imported Compose icon API.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.kt (1)
60-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe bracket stroke is clipped at the canvas edge.
Canvasclips its content to its own bounds. The corner brackets are drawn on the exact boundary atOffset(0f, 0f)andOffset(w, h). A stroke is centered on its path, so half of the 6.dp width, 3.dp, falls outside the bounds and is cut off. The brackets then appear thinner on the outer side than intended.Inset the drawing by half of the stroke width.
♻️ Proposed refactor
val stroke = 6.dp.toPx() val arm = 32.dp.toPx() val radius = 16.dp.toPx() - val w = size.width - val h = size.height + val inset = stroke / 2f + val w = size.width - inset + val h = size.height - inset val corners = listOf( - Offset(0f, 0f) to (1f to 1f), // top-left + Offset(inset, inset) to (1f to 1f), // top-left Offset(w, 0f) to (-1f to 1f), // top-right Offset(w, h) to (-1f to -1f), // bottom-right - Offset(0f, h) to (1f to -1f), // bottom-left + Offset(inset, h) to (1f to -1f), // bottom-left )Also change the
yof the top-right pivot and thexof the bottom-right pivot to useinsetwhere they sit on the top or left edge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.kt` around lines 60 - 93, Update the Canvas bracket drawing in the corners loop to inset every pivot by half the stroke width, keeping the full centered stroke inside the canvas. Apply the inset to both coordinates where a corner lies on an edge, including the top-right y coordinate and bottom-right x coordinate, while preserving the existing bracket directions and lengths.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/LoadingIndicator.kt (1)
44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not hardcode the text color in a reusable component.
Color.Whiteis readable on the gray camera background inQrScannerScreen, but it becomes invisible on a light surface.LoadingIndicatoris a general-purpose component. Expose the color as a parameter that defaults to a theme color.♻️ Proposed refactor
fun LoadingIndicator( message: String, - modifier: Modifier = Modifier + modifier: Modifier = Modifier, + contentColor: Color = MaterialTheme.colorScheme.onSurface, ) { @@ - CircularProgressIndicator() + CircularProgressIndicator(color = contentColor) Spacer(modifier = Modifier.height(16.dp)) Text( text = message, style = MaterialTheme.typography.bodyLarge, - color = Color.White + color = contentColor )Pass
contentColor = Color.Whiteat theQrScannerScreencall site to keep the current appearance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/LoadingIndicator.kt` around lines 44 - 50, Update the reusable LoadingIndicator component to expose a content-color parameter with a theme-color default, and use it for the message Text instead of hardcoded Color.White. At the QrScannerScreen call site, explicitly pass Color.White to preserve its existing appearance.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.kt (1)
138-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the account guard.
firstOrNull()?.let { ... }discards the account and only tests for a non-empty list. Use an explicit emptiness check to state the intent.♻️ Proposed refactor
- if (isActive && !uiState.isRefreshingOtp) { - uiState.accounts.firstOrNull()?.let { viewModel.generateOtp() } - } + if (isActive && !uiState.isRefreshingOtp && uiState.accounts.isNotEmpty()) { + viewModel.generateOtp() + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.kt` around lines 138 - 140, Update the account guard in AccountsScreen so the generateOtp call is gated by an explicit non-empty check on uiState.accounts instead of using firstOrNull()?.let, since the account value is unused. Keep the existing isActive and !uiState.isRefreshingOtp conditions unchanged, and preserve the same behavior of only calling viewModel.generateOtp() when at least one account exists.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualPairingPanel.kt (1)
40-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApply the Compose API conventions.
Three small items in this signature and body:
- Line 42:
modifierprecedes the required parameters. The Compose API guidelines placemodifieras the first optional parameter, after all required parameters.- Lines 66-67: the code uses the fully qualified
androidx.compose.material3.MaterialTheme. Import the symbol instead.- Line 76:
12is a magic number. Extract it to a named constant.♻️ Proposed refactor
+import androidx.compose.material3.MaterialTheme + +private const val MIN_PAIRING_KEY_LENGTH = 12 + `@Composable` fun ManualPairingPanel( - modifier: Modifier = Modifier, value: String, onValueChange: (String) -> Unit, onPair: (String) -> Unit, + modifier: Modifier = Modifier, isPairing: Boolean = false, ) { @@ colors = OutlinedTextFieldDefaults.colors( - unfocusedContainerColor = androidx.compose.material3.MaterialTheme.colorScheme.surface, - focusedContainerColor = androidx.compose.material3.MaterialTheme.colorScheme.surface, + unfocusedContainerColor = MaterialTheme.colorScheme.surface, + focusedContainerColor = MaterialTheme.colorScheme.surface, ), @@ - enabled = value.trim().length >= 12 && !isPairing, + enabled = value.trim().length >= MIN_PAIRING_KEY_LENGTH && !isPairing,All call sites use named arguments, so the parameter reorder is source compatible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualPairingPanel.kt` around lines 40 - 76, Update ManualPairingPanel so required parameters precede the optional modifier parameter, then import MaterialTheme and use it directly for both container color references. Extract the pairing-length threshold 12 into a descriptive named constant and use that constant in the enabled condition.android/kotlin-pingonemfa/app/src/main/res/drawable/ping_logo.xml (1)
8-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce the logo vector cost and confirm the unused animation group.
Three points on this asset:
- Intrinsic size is
794dpsquare. That size only applies when a caller sets no size modifier, and it produces a very large default. Setandroid:width/android:heightto the intended display size (for example120dp) and keep the794viewport.- The second path begins with the full-canvas rectangle
M1,396.5L1,793 397,793L793,793 793,396.5L793,0 397,0L1,0 1,396.5and then repeats the same glyph geometry as the first path. If the second path paints over the first, the first path is dead weight and doubles the rasterization cost.- The group is named
animationGroupand declarespivotX/pivotY, but noAnimatedVectorDrawablein this cohort targets it. If no animator exists, remove the group.Run the following script to check for an animator that targets
animationGroupand for the display sizes used by callers:#!/bin/bash # Description: Find animators targeting `animationGroup` and all usages of `ping_logo`. set -euo pipefail echo "=== animated-vector / objectAnimator resources ===" fd -e xml . android/kotlin-pingonemfa/app/src/main/res --exec rg -l 'animated-vector|objectAnimator' {} \; || true echo "=== references to animationGroup ===" rg -n 'animationGroup' android/kotlin-pingonemfa || true echo "=== usages of ping_logo and nearby size modifiers ===" rg -n -C 4 'ping_logo' android/kotlin-pingonemfa || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/res/drawable/ping_logo.xml` around lines 8 - 27, Reduce ping_logo’s intrinsic width and height to the intended caller display size while preserving the 794 viewport, then inspect usages and animator resources for animationGroup. If no animator targets animationGroup, remove the group wrapper and its pivot/name attributes; also remove the redundant second path when its full-canvas paint and duplicated glyph geometry make the first path unused, preserving the final rendered logo.android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThree copied Material icons miss the project copyright header, and two pin a fixed white tint. The three assets were added from the Material icon set without the MIT header that the other new resources in this PR carry. Two of them also keep
android:tint="#FFFFFF", which blocks theming at the call site.
android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml#L6-L9: add the MIT copyright header and removeandroid:tint="#FFFFFF"; tint the icon where it is drawn.android/kotlin-pingonemfa/app/src/main/res/drawable/ic_close.xml#L6-L9: add the MIT copyright header and removeandroid:tint="#FFFFFF"; tint the icon where it is drawn.android/kotlin-pingonemfa/app/src/main/res/drawable/ic_notification.xml#L1-L9: add the MIT copyright header. Keep the white fill, because Android renders the small notification icon as an alpha silhouette.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml` around lines 6 - 9, Add the missing MIT copyright header to the three copied Material icon drawables at android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml#L6-L9, android/kotlin-pingonemfa/app/src/main/res/drawable/ic_close.xml#L6-L9, and android/kotlin-pingonemfa/app/src/main/res/drawable/ic_notification.xml#L1-L9; also remove the fixed android:tint="`#FFFFFF`" from ic_check.xml and ic_close.xml so those icons can be tinted at the call site, while leaving the white fill behavior in ic_notification.xml unchanged.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.kt (1)
105-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the dropped notification when notifications are disabled.
If
areNotificationsEnabled()returnsfalse, this method returns without posting and without recording anything. The background push is then lost with no trace, andPushNotificationStorestays empty, so a later cancel push also finds nothing. Add a diagnostic log for this branch so the sample surfaces the cause.♻️ Proposed change
with(NotificationManagerCompat.from(context)) { if (areNotificationsEnabled()) { // Record in-process only once we know the banner will actually be posted. PushNotificationStore.put(notification) notify(notificationId, builder.build()) + } else { + DiagnosticLogger.w( + "Notifications are disabled — dropped push ${notification.id}", + null + ) } }Add the import:
import com.pingidentity.samples.pingonesample.data.DiagnosticLogger🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.kt` around lines 105 - 111, Add DiagnosticLogger to the notification helper and log a diagnostic message in the false branch of areNotificationsEnabled(), including enough context to identify the dropped notification. Keep the existing PushNotificationStore.put and notify behavior unchanged when notifications are enabled.android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.kt (1)
42-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared approve/deny execution flow.
approveanddenyrepeat the same structure: the loading guard, theisLoadingupdate, theonSuccess/onFailurehandling, and thecatchblock. Only the SDK call and the three strings differ. Extract a private helper that takes the success title, the success message, the failure fallback, and a suspending action. This removes about 40 duplicated lines and keeps the two paths consistent when one changes.♻️ Sketch of the extraction
private fun run( successTitle: String, successMessage: String, failureFallback: String, action: suspend () -> Result<Unit>, ) { if (_uiState.value.isLoading) return viewModelScope.launch { _uiState.update { it.copy(isLoading = true) } val dialogState = try { action().fold( onSuccess = { PushDialogState.Success(successTitle, successMessage) }, onFailure = { e -> DiagnosticLogger.e("PushNotificationViewModel: failed — ${e.message}", e) PushDialogState.Error(e.message ?: failureFallback) }, ) } catch (e: Exception) { DiagnosticLogger.e("PushNotificationViewModel: threw — ${e.message}", e) PushDialogState.Error(e.message ?: failureFallback) } _uiState.update { it.copy(isLoading = false, dialogState = dialogState) } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.kt` around lines 42 - 144, Extract the duplicated execution logic from approve and deny into a private helper in PushNotificationViewModel that accepts the success title, success message, failure fallback, and a suspending Result<Unit> action. Move the loading guard, viewModelScope launch, loading-state updates, success/failure handling, and exception handling into this helper, then have approve and deny provide only their strings and SDK actions while preserving existing logging and dialog behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/DiagnosticLogger.kt`:
- Around line 43-44: Update DiagnosticLogger’s shared dateFormat usage so every
dateFormat.format call in addLogEntry and exportLogs is protected by the same
lock, or replace dateFormat with a thread-safe formatter while preserving the
existing timestamp format.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/PingOneViewModel.kt`:
- Around line 124-132: Update the generateOtp failure handler in
PingOneViewModel so it clears both generatedCode and otpExpiresAtElapsedMs when
refreshing fails, preventing an expired OTP from remaining in UI state. Preserve
the existing error, isRefreshingOtp, and otpVersion updates; only set a new
explicit retry deadline if the surrounding retry behavior requires automatic
retry.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationActionReceiver.kt`:
- Around line 57-61: Update NotificationActionReceiver.onReceive so
PushNotificationStore.remove() and banner cancellation occur only for recognized
ACTION_APPROVE and ACTION_DENY actions. Move both side effects into those
branches, reusing a dismiss helper if appropriate, and leave unrecognized
actions untouched so the pending request remains answerable.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.kt`:
- Around line 128-131: Update PushNotificationService.onDestroy to cancel the
coroutine scope before or after calling super.onDestroy, using the
kotlinx.coroutines.cancel extension, so all coroutines launched from
onMessageReceived, onNewToken, and handleNotification are stopped when the
service is destroyed.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.kt`:
- Around line 122-141: Update the countdown logic in the LaunchedEffect keyed by
uiState.otpVersion so a null otpExpiresAtElapsedMs creates one fixed deadline
using DEFAULT_OTP_TTL_SECONDS from the effect start time. Make
computeSecsRemaining() derive remaining time from that captured deadline on
every tick, allowing secsRemaining to reach zero and the existing generateOtp()
retry to execute.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/AccountAvatar.kt`:
- Around line 77-80: Update generateAvatarColor so hue uses Math.floorMod on
seed.hashCode() with 360, guaranteeing a non-negative value even for
Int.MIN_VALUE; remove the unused kotlin.math.absoluteValue import.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualNumberChallenge.kt`:
- Around line 57-77: Update the input handling in the OutlinedTextField to
accept digits only up to 10 characters, preventing values that
input.toIntOrNull() cannot convert to an Int. Preserve the existing button
behavior for valid non-empty input.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.kt`:
- Around line 52-57: Update the title Text in QrGrid to use a distinct
camera-overlay instruction instead of R.string.qr_scanner_title, while leaving
QrScannerScreen's BackNavigationTopAppBar label unchanged. Add or reuse an
appropriate localized string such as “Point the camera at the QR code.”
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/QrScannerScreen.kt`:
- Around line 93-116: Update the permission state handling in QrScannerScreen
around hasCameraPermission and LaunchedEffect so the current CAMERA permission
is re-checked on every Lifecycle.Event.ON_RESUME, including changes made in
system settings. Use the lifecycle resume callback to refresh
hasCameraPermission and preserve the existing request flow for denied
permissions.
- Around line 302-309: Fix the teardown ordering so the camera stops delivering
frames before the ML Kit scanner is released: in QrScannerScreen’s
DisposableEffect onDispose, unbind all CameraX use cases with
ProcessCameraProvider.getInstance(context).get().unbindAll() wrapped in
runCatching before calling qrAnalyzer.close() and cameraExecutor.shutdown();
then in QrCodeAnalyzer, add a closed flag that is set in close() before
scanner.close(), and make analyze() exit early after closing the ImageProxy when
that flag is already set so no task can reach scanner.process() after disposal.
- Around line 225-227: Update the exception handling in QrScannerScreen’s camera
error path to provide a non-null fallback when formatting e.message, preventing
the user-facing error from containing the literal “null”. Preserve the existing
cameraErrorTemplate and viewModel.setError flow.
In `@android/kotlin-pingonemfa/gradle/libs.versions.toml`:
- Line 3: Update the kotlin version in the version catalog to a Kotlin release
supported by the project’s declared AGP 9.1.1 and Gradle 9.3.1 versions, and
ensure the org.jetbrains.kotlin.plugin.compose version follows the same
supported Kotlin release. Keep both Kotlin and Compose compiler versions aligned
with the selected AGP.
In `@android/kotlin-pingonemfa/README.md`:
- Around line 14-20: Update the “Getting Started” steps in the README to include
running ./gradlew publishToMavenLocal from the SDK repository root before
opening or building the sample app, then preserve the existing configuration and
build steps.
---
Nitpick comments:
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.kt`:
- Around line 105-111: Add DiagnosticLogger to the notification helper and log a
diagnostic message in the false branch of areNotificationsEnabled(), including
enough context to identify the dropped notification. Keep the existing
PushNotificationStore.put and notify behavior unchanged when notifications are
enabled.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.kt`:
- Around line 42-144: Extract the duplicated execution logic from approve and
deny into a private helper in PushNotificationViewModel that accepts the success
title, success message, failure fallback, and a suspending Result<Unit> action.
Move the loading guard, viewModelScope launch, loading-state updates,
success/failure handling, and exception handling into this helper, then have
approve and deny provide only their strings and SDK actions while preserving
existing logging and dialog behavior.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.kt`:
- Around line 138-140: Update the account guard in AccountsScreen so the
generateOtp call is gated by an explicit non-empty check on uiState.accounts
instead of using firstOrNull()?.let, since the account value is unused. Keep the
existing isActive and !uiState.isRefreshingOtp conditions unchanged, and
preserve the same behavior of only calling viewModel.generateOtp() when at least
one account exists.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/LoadingIndicator.kt`:
- Around line 44-50: Update the reusable LoadingIndicator component to expose a
content-color parameter with a theme-color default, and use it for the message
Text instead of hardcoded Color.White. At the QrScannerScreen call site,
explicitly pass Color.White to preserve its existing appearance.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualPairingPanel.kt`:
- Around line 40-76: Update ManualPairingPanel so required parameters precede
the optional modifier parameter, then import MaterialTheme and use it directly
for both container color references. Extract the pairing-length threshold 12
into a descriptive named constant and use that constant in the enabled
condition.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/OtpBox.kt`:
- Line 42: Update the KDoc description in OtpBox to replace the unresolved [Tag]
reference with plain text or a valid Icons.Default.Tag reference, matching the
imported Compose icon API.
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.kt`:
- Around line 60-93: Update the Canvas bracket drawing in the corners loop to
inset every pivot by half the stroke width, keeping the full centered stroke
inside the canvas. Apply the inset to both coordinates where a corner lies on an
edge, including the top-right y coordinate and bottom-right x coordinate, while
preserving the existing bracket directions and lengths.
In `@android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml`:
- Around line 6-9: Add the missing MIT copyright header to the three copied
Material icon drawables at
android/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xml#L6-L9,
android/kotlin-pingonemfa/app/src/main/res/drawable/ic_close.xml#L6-L9, and
android/kotlin-pingonemfa/app/src/main/res/drawable/ic_notification.xml#L1-L9;
also remove the fixed android:tint="`#FFFFFF`" from ic_check.xml and ic_close.xml
so those icons can be tinted at the call site, while leaving the white fill
behavior in ic_notification.xml unchanged.
In `@android/kotlin-pingonemfa/app/src/main/res/drawable/ping_logo.xml`:
- Around line 8-27: Reduce ping_logo’s intrinsic width and height to the
intended caller display size while preserving the 794 viewport, then inspect
usages and animator resources for animationGroup. If no animator targets
animationGroup, remove the group wrapper and its pivot/name attributes; also
remove the redundant second path when its full-canvas paint and duplicated glyph
geometry make the first path unused, preserving the final rendered logo.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 710994ac-a28d-40fe-ac58-bdc2691e6523
📒 Files selected for processing (55)
android/kotlin-pingonemfa/.gitignoreandroid/kotlin-pingonemfa/README.mdandroid/kotlin-pingonemfa/app/.gitignoreandroid/kotlin-pingonemfa/app/build.gradle.ktsandroid/kotlin-pingonemfa/app/proguard-rules.proandroid/kotlin-pingonemfa/app/src/main/AndroidManifest.xmlandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/AuthApp.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/MainActivity.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/PingOneSampleApplication.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/DiagnosticLogger.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/PingOneViewModel.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationActionReceiver.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationCancelBus.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationActivity.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationStore.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/theme/Color.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/theme/Shape.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/theme/Theme.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/theme/Type.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/DiagnosticLogsScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/PushNotificationScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/QrScannerScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/AccountAvatar.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/AccountCard.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ApproveDenyRow.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/BackNavigationTopAppBar.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/LoadingIndicator.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualNumberChallenge.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualPairingPanel.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/NumberChallengeOptions.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/OtpBox.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/util/QrCodeAnalyzer.ktandroid/kotlin-pingonemfa/app/src/main/res/drawable/ic_check.xmlandroid/kotlin-pingonemfa/app/src/main/res/drawable/ic_close.xmlandroid/kotlin-pingonemfa/app/src/main/res/drawable/ic_launcher_background.xmlandroid/kotlin-pingonemfa/app/src/main/res/drawable/ic_launcher_foreground.xmlandroid/kotlin-pingonemfa/app/src/main/res/drawable/ic_notification.xmlandroid/kotlin-pingonemfa/app/src/main/res/drawable/ping_logo.xmlandroid/kotlin-pingonemfa/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xmlandroid/kotlin-pingonemfa/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xmlandroid/kotlin-pingonemfa/app/src/main/res/values/colors.xmlandroid/kotlin-pingonemfa/app/src/main/res/values/strings.xmlandroid/kotlin-pingonemfa/app/src/main/res/values/themes.xmlandroid/kotlin-pingonemfa/build.gradle.ktsandroid/kotlin-pingonemfa/gradle.propertiesandroid/kotlin-pingonemfa/gradle/libs.versions.tomlandroid/kotlin-pingonemfa/gradle/wrapper/gradle-wrapper.propertiesandroid/kotlin-pingonemfa/gradlewandroid/kotlin-pingonemfa/gradlew.batandroid/kotlin-pingonemfa/settings.gradle.kts
New Android Kotlin/Compose sample demonstrating PingOne MFA SDK integration: QR-code account pairing, TOTP display with live countdown, and push-notification approve/deny flows (default, number-challenge, dry-run, and server-cancel). Key implementation details: - PingOneSampleApplication performs one-time SDK init and FCM token registration in a process-lifetime CoroutineScope, sequentially (token registration runs only after initialize() succeeds) so the device is always registered for push on first install. - PushNotificationService forks foreground vs. background on arrival: foreground launches PushNotificationActivity directly via startActivity (BAL-safe because ProcessLifecycleOwner confirms foreground); background posts a high-priority notification with setFullScreenIntent so the OS wakes the screen from lock without the service calling startActivity. - PushNotification is carried as a Parcelable Intent extra so PushNotificationActivity survives process death between banner post and user tap. PushNotificationStore is narrowed to in-process cancel-path coordination only. - NotificationCancelBus uses MutableSharedFlow(replay=1) so a cancel signal emitted before PushNotificationActivity subscribes is not dropped. - PushNotificationViewModel owns approve/deny in viewModelScope so in-flight SDK calls survive Activity recreation. onNewIntent swaps the displayed notification via Compose state (no recreate()) and calls resetState() to clear stale dialog results atomically. - OTP countdown uses a SystemClock.elapsedRealtime() deadline anchored at SDK response time and stored in ViewModel, so the counter resumes at the true remaining time after navigation rather than resetting to 30 s. otpVersion counter guarantees LaunchedEffect restarts on every generateOtp() outcome including failure. - QrCodeAnalyzer is a single remembered instance with DisposableEffect cleanup; scanned guard uses AtomicBoolean for thread-safe compare-and-set between camera executor and composition threads. CameraProvider binding uses addListener(ContextCompat.getMainExecutor) to avoid blocking the main thread and satisfy CameraX @mainthread assertions. - All strings externalised to strings.xml; MIT copyright headers on every source file; DiagnosticLogger StateFlow updates use update() for atomicity.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.kt (1)
61-70: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep FCM work within the callback execution window.
FirebaseMessagingService.onMessageReceived()andonNewToken()return without keeping the Android process alive, andscope.launchin both callbacks leaves PingOne MFA work running after the callback ends. Process these calls synchronously before returning, or schedule durable/long-running work withWorkManagerinstead of relying on the instance-owned coroutine scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.kt` around lines 61 - 70, Update the FirebaseMessagingService callbacks that invoke PingOneMFA processing to complete the work before returning, replacing the instance-owned scope.launch usage with synchronous execution, or enqueueing durable long-running work through WorkManager. Apply this to both onMessageReceived() and onNewToken(), preserving the existing success and failure handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.kt`:
- Around line 61-70: Update the FirebaseMessagingService callbacks that invoke
PingOneMFA processing to complete the work before returning, replacing the
instance-owned scope.launch usage with synchronous execution, or enqueueing
durable long-running work through WorkManager. Apply this to both
onMessageReceived() and onNewToken(), preserving the existing success and
failure handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a4a4daa6-5d28-4001-a173-e25cb7580bf2
📒 Files selected for processing (20)
android/kotlin-pingonemfa/app/build.gradle.ktsandroid/kotlin-pingonemfa/app/google-services.jsonandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/MainActivity.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/PingOneSampleApplication.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/DiagnosticLogger.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/PingOneViewModel.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationActionReceiver.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationActivity.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/service/PushNotificationService.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/AccountsScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/PushNotificationScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/QrScannerScreen.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/AccountAvatar.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualNumberChallenge.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.ktandroid/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/util/QrCodeAnalyzer.ktandroid/kotlin-pingonemfa/gradle/libs.versions.tomlandroid/kotlin-pingonemfa/gradle/wrapper/gradle-wrapper.properties
🚧 Files skipped from review as they are similar to previous changes (16)
- android/kotlin-pingonemfa/gradle/wrapper/gradle-wrapper.properties
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationActivity.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/QrGrid.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/MainActivity.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/PushNotificationViewModel.kt
- android/kotlin-pingonemfa/gradle/libs.versions.toml
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/PushNotificationScreen.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/AccountAvatar.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/components/ManualNumberChallenge.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/notification/NotificationHelper.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/PingOneSampleApplication.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/PingOneViewModel.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/ui/QrScannerScreen.kt
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/data/DiagnosticLogger.kt
- android/kotlin-pingonemfa/app/build.gradle.kts
- android/kotlin-pingonemfa/app/src/main/java/com/pingidentity/samples/pingonesample/util/QrCodeAnalyzer.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prerequisites:
Until the sdk isn't released to MavenCentral deploy the PingOne MFA SDK to your local Maven repository:
New Android Kotlin/Compose sample demonstrating PingOne MFA SDK integration: QR-code account pairing, TOTP display with live countdown, and push-notification approve/deny flows (default, number-challenge, dry-run, and server-cancel).
Key implementation details:
PingOneSampleApplication performs one-time SDK init and FCM token registration in a process-lifetime CoroutineScope, sequentially (token registration runs only after initialize() succeeds) so the device is always registered for push on first install.
PushNotificationService forks foreground vs. background on arrival: foreground launches PushNotificationActivity directly via startActivity (BAL-safe because ProcessLifecycleOwner confirms foreground); background posts a high-priority notification with setFullScreenIntent so the OS wakes the screen from lock without the service calling startActivity.
PushNotification is carried as a Parcelable Intent extra so PushNotificationActivity survives process death between banner post and user tap. PushNotificationStore is narrowed to in-process cancel-path coordination only.
NotificationCancelBus uses MutableSharedFlow(replay=1) so a cancel signal emitted before PushNotificationActivity subscribes is not dropped.
PushNotificationViewModel owns approve/deny in viewModelScope so in-flight SDK calls survive Activity recreation. onNewIntent swaps the displayed notification via Compose state (no recreate()) and calls resetState() to clear stale dialog results atomically.
OTP countdown uses a SystemClock.elapsedRealtime() deadline anchored at SDK response time and stored in ViewModel, so the counter resumes at the true remaining time after navigation rather than resetting to 30 s. otpVersion counter guarantees LaunchedEffect restarts on every generateOtp() outcome including failure.
QrCodeAnalyzer is a single remembered instance with DisposableEffect cleanup; scanned guard uses AtomicBoolean for thread-safe compare-and-set between camera executor and composition threads. CameraProvider binding uses addListener(ContextCompat.getMainExecutor) to avoid blocking the main thread and satisfy CameraX @mainthread assertions.
All strings externalised to strings.xml; MIT copyright headers on every source file; DiagnosticLogger StateFlow updates use update() for atomicity.
Summary by CodeRabbit
New Features
Documentation