[Connect SDK] Add camera support to the Connect SDK #9607

simond-stripe · 2024-11-13T00:36:23Z

Summary

Hooks the webview's camera permission into the Android app's camera permission so that the user is shown a permissions modal when the connect SDK requests a camera permission.

MXMOBILE-2913

Motivation

Certain embedded components (particularly any that use Identity) make use of the camera, and the SDK needs to properly handle camera permissions to make sure these features work.

Testing

Added tests
Modified tests
Manually verified

The easiest way I found to test was:

Go to payouts
Select "Payout" button
Select "Add account" under the dropdown of Stripe accounts to select
Select an existing account and click the "...", then "edit"
Click "you can verify your identity instead"
This opens the Identity screen, at which point you can go through the identity flow with the camera being used to scan an ID (in test mode select "Proceed" under "Preview user experience" to make sure you get the camera pop-up)

Screenshots

Granting permission

demo-1733780574.mp4

Rejecting permission

demo-1733780639.mp4

github-actions · 2024-11-13T00:47:42Z

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.1 MiB │   4.1 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 301.8 KiB │ 301.8 KiB │  0 B │ 455.5 KiB │ 455.5 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.1 KiB │   7.1 KiB │  0 B │   6.9 KiB │   6.9 KiB │  0 B 
    other │  90.2 KiB │  90.2 KiB │ -7 B │ 170.3 KiB │ 170.3 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.6 MiB │   9.6 MiB │ -7 B │  21.5 MiB │  21.5 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 19966 │ 19966 │ 0 (+0 -0) 
   types │  6188 │  6188 │ 0 (+0 -0) 
 classes │  4979 │  4979 │ 0 (+0 -0) 
 methods │ 29759 │ 29759 │ 0 (+0 -0) 
  fields │ 17526 │ 17526 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3622 │ 3622 │  0

APK

   compressed    │   uncompressed   │                                           
──────────┬──────┼───────────┬──────┤                                           
 size     │ diff │ size      │ diff │ path                                      
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 28.4 KiB │ -6 B │  62.9 KiB │  0 B │ ∆ META-INF/CERT.SF                        
    269 B │ -3 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
 25.3 KiB │ +3 B │  62.8 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
  1.2 KiB │ -1 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 55.2 KiB │ -7 B │ 127.1 KiB │  0 B │ (total)

# Conflicts: # connect-example/src/main/java/com/stripe/android/connect/example/ui/features/payouts/PayoutsExampleActivity.kt # connect/src/main/java/com/stripe/android/connect/PayoutsView.kt # Conflicts: # connect/src/main/java/com/stripe/android/connect/PayoutsView.kt # connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewClient.kt # Conflicts: # connect-example/src/main/java/com/stripe/android/connect/example/ui/features/payouts/PayoutsExampleActivity.kt # connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt # connect/src/main/java/com/stripe/android/connect/PayoutsView.kt

# Conflicts: # connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainer.kt

* Show promo badge in bank form * Address code review feedback Fix layout issue with super-long bank name and validate with screenshot test.

…#9727) * Add Embedded Appearance params to AppearanceBottomSheetDialogFragment

* update text style for bacs secondary button type * screenshots for screenshot tests * Apply suggestions from code review Capitalize comment and add period Co-authored-by: Bella Koch <160939932+amk-stripe@users.noreply.github.com>

…ltConfirmationHandler` (#9754)

lng-stripe

Very cool to see this working!

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

lng-stripe · 2024-12-11T16:34:22Z

.../src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerController.kt

+    /**
+     *
+     */
+    suspend fun onPermissionRequest(context: Context, request: PermissionRequest) {


fix docstring

.../src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerController.kt

lng-stripe · 2024-12-11T16:39:37Z

.../src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerController.kt

+            logger.debug(
+                "($loggerTag) Denying permission - ${request.resources.joinToString()} are not supported"
+            )


should this be warning log level?

Good call. We'll want an analytic event too now that I think about it so that we know if an embedded component is expecting any unsupported permissions. I've added a TODO for when we add analytics (hopefully tomorrow or Friday)

lng-stripe · 2024-12-11T16:50:05Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

+    internal suspend fun requestCameraPermission(context: Context): Boolean? {
+        val activity = context.findActivity() ?: error("You must create an AccountOnboardingView from an Activity")
+        launcherMap[activity]?.launch(android.Manifest.permission.CAMERA) ?: return null


I'm concerned throwing here would be too user-unfriendly since it's pretty late in the UX flow to be crashing. I think instead we should return null and log warnings if either the activity or launcher are null.

Separately, the log message should be generalized and not reference a specific view, "AccountOnboardingView" in this case.

I agree it's a worse bug to crash than it is to let the user proceed but with broken permissions. What do you think about crashing if in debug (since this really is a programmer error), but then following the behavior you outlined otherwise?

lng-stripe · 2024-12-11T17:02:08Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

+            activity.registerForActivityResult(
+                ActivityResultContracts.RequestPermission()
+            ) { isGranted ->
+                MainScope().launch {
+                    permissionsFlow.emit(isGranted)
+                }
+            }.also {
+                launcherMap[activity] = it
+            }


Could simplify the assignment and maybe use tryEmit instead of launching. We might need to configure the MutableSharedFlow with extraBufferCapacity = 1 though.

Suggested change

activity.registerForActivityResult(

ActivityResultContracts.RequestPermission()

) { isGranted ->

MainScope().launch {

permissionsFlow.emit(isGranted)

}

}.also {

launcherMap[activity] = it

}

launcherMap[activity] =

activity.registerForActivityResult(

ActivityResultContracts.RequestPermission()

) { isGranted ->

permissionsFlow.tryEmit(isGranted)

}

I like this suggestion. Is there any risk of race conditions where multiple tryEmits are called before the consumers from permissionFlow.first get to them? I think the risk is low and acceptable, curious on your thoughts @lng-stripe

connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainer.kt

lng-stripe · 2024-12-11T17:09:17Z

.../src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerController.kt

+            return
+        }
+
+        if (checkSelfPermission(context, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) {


Make EmbeddedComponentManager fully responsible for the native camera permission and change this to

Suggested change

if (checkSelfPermission(context, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) {

if (embeddedComponentManager.hasCameraPermission(context)) {

otherwise this code risks becoming out of sync with embeddedComponentManager.requestCameraPermission(context)

Good call. @lng-stripe Thoughts on having a single call to EmbeddedComponentManager and inlining this into requestCameraPermission()? ie:

internal suspend fun requestCameraPermission(context: Context): Boolean? { if (checkSelfPermission(context, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) { logger.debug("($loggerTag) Skipping permission request - CAMERA permission already granted") return true } // proceed with explicit check... }

I think it's worth having a hasCameraPermission() function and your change above.

Like abstract the above if statement into a hasCameraPermission function, or keep what I have in that comment and also add a separate hasCameraPermission() call in the Controller? The latter feels duplicative to me

I think arguments could be made for either. What you have now is fine 👍

lng-stripe · 2024-12-11T17:13:07Z

.../src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerController.kt

+            request.grant(permissionsRequested)
+        } else {
+            val isGranted = embeddedComponentManager.requestCameraPermission(context) ?: return
+            withContext(Dispatchers.Main) {


If I'm reading the code correctly, either setting the dispatcher here isn't necessary, or we'd need to do so for the other request.deny() / grant() calls above (I think it's the former). Could you check?

Ah yeah, we do need this because the request/deny call crashes if not done on the Main thread. I re-confirmed by wrapping the parent call in withContext(Dispatchers.IO) { ... } - the reason I didn't catch the one on line 153 is that I moved the parent call to use the view lifecycle scope which is the main thread. For safety it's important this function guarantees it's done on the main thread - updated!

Agreed -- good point that it's safer to be explicit here

lng-stripe · 2024-12-11T17:17:13Z

.../test/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerControllerTest.kt

@@ -58,6 +66,10 @@ class StripeConnectWebViewContainerControllerTest {

    @Before
    fun setup() {
+        Dispatchers.setMain(Dispatchers.Unconfined)


Shouldn't be necessary if we remove setting the dispatcher as suggested in my other comment. Otherwise, we'll need to Dispatchers.resetMain() in an @After block.

This is still necessary - see comment above. I've added an @After as well now

…eConnectWebViewContainer.kt Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

…nentManager.kt Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

lng-stripe · 2024-12-12T15:08:11Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

+    private val logger: Logger = Logger.getInstance(enableLogging = BuildConfig.DEBUG),
+    private val isDebugBuild: Boolean = BuildConfig.DEBUG,


We shouldn't add this to the public API, which EmbeddedComponentManager and this constructor will be part of (we should remove the @RestrictTo above). I do think we should add a debug: Boolean property to Configuration, but logger should not be exposed and instead should be instantiated internally.

lng-stripe · 2024-12-12T15:19:08Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

+            val application = activity.application
+
+            application.registerActivityLifecycleCallbacks(object : ActivityLifecycleCallbacks {


Why register on the Application? I think it was better before to register on the Activity. Reading the docs, it turns out you actually don't need to unregister the callback:

As this callback is associated with only this Activity, it is not usually necessary to unregister it unless you specifically do not want to receive further lifecycle callbacks.

I also prefer the callbacks on the activity, but that API is only available on API 29+ (whereas the Application version is available on 14+): https://developer.android.com/reference/android/app/Activity#registerActivityLifecycleCallbacks(android.app.Application.ActivityLifecycleCallbacks)

lng-stripe · 2024-12-12T15:21:14Z

connect/src/test/java/com/stripe/android/connect/EmbeddedComponentManagerTest.kt


    @Before
    fun setup() {
+        Dispatchers.setMain(Dispatchers.Unconfined)


Need to resetMain() in this test class, too

Oops, I don't even need this dispatcher and forgot to remove it 😅

lng-stripe · 2024-12-12T15:21:52Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt


    /**
     * Create a new [AccountOnboardingView] for inclusion in the view hierarchy.
     */
+    @PrivateBetaConnectSDK
+    @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)


All these functions should not be restricted to the library -- they will be part of the public API. Also, isn't @PrivateBetaConnectSDK redundant when it's already on the class?

Yeah I didn't realize that this is only needed at the class level, so I cleaned it up throughout the library.

We're adding @RestrictTo until we're ready to go into private beta, at which point we'll remove it and external developers will be able to use these classes (with the right opt-in). See here: #9203 (comment)

lng-stripe · 2024-12-12T15:23:07Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

@@ -42,17 +70,40 @@ class EmbeddedComponentManager(
    /**
     * Create a new [PayoutsView] for inclusion in the view hierarchy.
     */
+    @PrivateBetaConnectSDK
+    @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)


ditto -- see other comment

lng-stripe

👍 👍

simond-stripe force-pushed the simond/camera-support-connect branch from 69616a3 to 16db6f3 Compare November 13, 2024 00:41

simond-stripe force-pushed the simond/camera-support-connect branch 2 times, most recently from 9e85b34 to 3603b54 Compare November 21, 2024 19:01

simond-stripe added 7 commits December 4, 2024 13:37

Update request code

9cbf28e

Update example activity

7ed75ef

Refactor permission, handle within manager

353a740

Fix leftover merge conflicts

61c3865

# Conflicts: # connect/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainer.kt

WIP

2b4743d

Fix merge

45f9c94

simond-stripe force-pushed the simond/camera-support-connect branch from 3603b54 to 45f9c94 Compare December 4, 2024 18:41

simond-stripe and others added 14 commits December 4, 2024 14:20

Fix build issues

8286739

fix activity

26f9fb9

Fix suspend function

c15db55

Merge branch 'master' into simond/camera-support-connect

1f9e9ab

refactor with activity oncreate

c0b4e5f

Show promo badge in bank form (#9734)

48b43fe

* Show promo badge in bank form * Address code review feedback Fix layout issue with super-long bank name and validate with screenshot test.

Add Embedded Appearance params to AppearanceBottomSheetDialogFragment (…

1c8577d

…#9727) * Add Embedded Appearance params to AppearanceBottomSheetDialogFragment

crypto payment method (#9753)

5b75c3e

Better handle process death for all confirmation flow cases in `Defau…

1fca7e4

…ltConfirmationHandler` (#9754)

Merge branch 'master' into simond/camera-support-connect

db3d30d

Fix lints, remove unneeded cancellation function

510074a

Add tests

5967530

Fix tests

6d787a1

simond-stripe requested a review from lng-stripe December 9, 2024 21:47

simond-stripe marked this pull request as ready for review December 9, 2024 21:48

simond-stripe requested a review from a team as a code owner December 9, 2024 21:48

Add/fix logging

372df59

Fix lint from log line

e213dff

lng-stripe requested changes Dec 11, 2024

View reviewed changes

simond-stripe and others added 12 commits December 11, 2024 13:44

Fix main thread

9dea759

Update connect/src/main/java/com/stripe/android/connect/webview/Strip…

6d4191d

…eConnectWebViewContainer.kt Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

Update connect/src/main/java/com/stripe/android/connect/EmbeddedCompo…

565e894

…nentManager.kt Co-authored-by: lng-stripe <91862945+lng-stripe@users.noreply.github.com>

unsupported behavior to log warning

9ec0c6d

Fix docstrings

37eb90d

Update crashing behavior for camera permission request

869783e

Move functions around for clarity

37ae1bc

Update tests

cca158d

Update and fix tests

216a71a

Fix detekt lints

027f55c

Merge branch 'master' into simond/camera-support-connect

1fba443

Fix main activity

5135824

lng-stripe requested changes Dec 12, 2024

View reviewed changes

simond-stripe added 5 commits December 12, 2024 11:15

Merge branch 'master' into simond/camera-support-connect

7ebf52d

Remove unnecessary fragment, move logger out of constructor

db3f6a0

Clean up lib restriction

fce77c0

Remove unnecessary unconfined test dispatcher

3ff615e

Merge branch 'master' into simond/camera-support-connect

bb40ce0

lng-stripe previously approved these changes Dec 12, 2024

View reviewed changes

simond-stripe added 2 commits December 12, 2024 15:46

Update api

619e440

Lint fixes

8be954e

simond-stripe dismissed lng-stripe’s stale review via 8be954e December 12, 2024 21:43

lng-stripe approved these changes Dec 12, 2024

View reviewed changes

simond-stripe merged commit 3d66b3c into master Dec 12, 2024
13 checks passed

simond-stripe deleted the simond/camera-support-connect branch December 12, 2024 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Connect SDK] Add camera support to the Connect SDK #9607

[Connect SDK] Add camera support to the Connect SDK #9607

simond-stripe commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

lng-stripe left a comment

lng-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

lng-stripe Dec 11, 2024

simond-stripe Dec 11, 2024

lng-stripe Dec 12, 2024

lng-stripe Dec 12, 2024

simond-stripe Dec 12, 2024

lng-stripe Dec 12, 2024

simond-stripe Dec 12, 2024

lng-stripe Dec 12, 2024 •

edited

Loading

simond-stripe Dec 12, 2024

lng-stripe Dec 12, 2024

lng-stripe left a comment

	if (checkSelfPermission(context, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) {
	if (embeddedComponentManager.hasCameraPermission(context)) {

		private val logger: Logger = Logger.getInstance(enableLogging = BuildConfig.DEBUG),
		private val isDebugBuild: Boolean = BuildConfig.DEBUG,

		val application = activity.application

		application.registerActivityLifecycleCallbacks(object : ActivityLifecycleCallbacks {

[Connect SDK] Add camera support to the Connect SDK #9607

[Connect SDK] Add camera support to the Connect SDK #9607

Conversation

simond-stripe commented Nov 13, 2024 • edited Loading

Summary

Motivation

Testing

Screenshots

Granting permission

Rejecting permission

github-actions bot commented Nov 13, 2024 • edited Loading

lng-stripe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lng-stripe Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lng-stripe left a comment

Choose a reason for hiding this comment

simond-stripe commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

lng-stripe Dec 12, 2024 •

edited

Loading