Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Connect SDK] Support http/https pop-ups #9640

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class SettingsService @Inject constructor(@ApplicationContext context: Context)
fun getAppearanceIdFlow(): Flow<AppearanceInfo.AppearanceId?> {
return observePref(APPEARANCE_ID_KEY) { prefs, key ->
val appearanceId = prefs.getString(key, null) ?: return@observePref null
AppearanceInfo.AppearanceId.valueOf(appearanceId)
AppearanceInfo.AppearanceId.entries.firstOrNull { it.name == appearanceId }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 this fixes a crash if any appearance id's change and can't be found anymore (something I ran into myself)

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.stripe.android.connect.webview
import android.annotation.SuppressLint
import android.graphics.Bitmap
import android.webkit.JavascriptInterface
import android.webkit.WebResourceRequest
import android.webkit.WebView
import android.webkit.WebViewClient
import androidx.core.view.doOnAttach
Expand Down Expand Up @@ -32,13 +33,34 @@ import kotlinx.serialization.json.jsonObject
internal class StripeConnectWebViewClient(
private val embeddedComponentManager: EmbeddedComponentManager,
private val connectComponent: StripeEmbeddedComponent,
private val stripeIntentLauncher: StripeIntentLauncher = StripeIntentLauncherImpl(),
private val logger: Logger = Logger.getInstance(enableLogging = BuildConfig.DEBUG),
private val jsonSerializer: Json = Json {
ignoreUnknownKeys = true
explicitNulls = false
}
) : WebViewClient() {

override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest?): Boolean {
view ?: return false // the view shouldn't be null, but if it is then don't handle the request
val url = request?.url ?: return false // if the request isn't for a url, then don't handle it

return if (url.host in ALLOWLISTED_HOSTS) {
// TODO - add an analytic event here to track this unexpected behavior
logger.warning("(StripeConnectWebViewClient) Received pop-up for allow-listed host: $url")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a warning because we don't expect getting any of these given that messaging between windows on Android isn't supported? If this is the case, we should add a TODO here to add an analytic so we can setup alerts for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going off of this in the spec: If host is allowlisted (connect.stripe.com or connect-js.stripe.com), print warning (stub out todo to log analytic error) because this isn't supported

Is that still accurate @mludowise-stripe? 👍 on adding an analytic here

false // Allow the request to propagate so we open URL in WebView, but this is not an expected operation
} else if (url.scheme == "https" || url.scheme == "http") {
// open the URL in an external browser for safety and to preserve back navigation
logger.debug("(StripeConnectWebViewClient) Opening URL in external browser: $url")
stripeIntentLauncher.launchSecureExternalWebTab(view.context, url)
true // block the request since we're opening it in a secure external tab
} else {
// TODO - support non-http/https schemes.
logger.debug("(StripeConnectWebViewClient) Received unsupported pop-up request: $url")
true // block the request as it's currently unsupported
}
}

override fun onPageStarted(view: WebView, url: String?, favicon: Bitmap?) {
initJavascriptBridge(view)
}
Expand All @@ -47,6 +69,7 @@ internal class StripeConnectWebViewClient(
fun configureAndLoadWebView(webView: WebView) {
webView.apply {
webViewClient = this@StripeConnectWebViewClient
webChromeClient = StripeWebChromeClient()
settings.apply {
javaScriptEnabled = true
domStorageEnabled = true
Expand Down Expand Up @@ -220,5 +243,6 @@ internal class StripeConnectWebViewClient(
internal companion object {
private const val ANDROID_JS_INTERFACE = "Android"
private const val ANDROID_JS_INTERNAL_INTERFACE = "AndroidInternal"
private val ALLOWLISTED_HOSTS = setOf("connect.stripe.com", "connect-js.stripe.com")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.stripe.android.connect.webview

import android.content.Context
import android.net.Uri
import androidx.browser.customtabs.CustomTabsIntent

internal interface StripeIntentLauncher {
/**
* Launches [uri] in a secure external Android Custom Tab.
*/
fun launchSecureExternalWebTab(context: Context, uri: Uri)
}

internal class StripeIntentLauncherImpl : StripeIntentLauncher {
override fun launchSecureExternalWebTab(context: Context, uri: Uri) {
val customTabsIntent = CustomTabsIntent.Builder().build()
customTabsIntent.launchUrl(context, uri)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.stripe.android.connect.webview

import android.webkit.WebChromeClient
import android.webkit.WebViewClient

/**
* A [WebChromeClient] that provides additional functionality for Stripe Connect Embedded Component WebViews.
*
* This class is currently empty, but it could be used to add additional functionality in the future
* Setting a [WebChromeClient] (even an empty one) is necessary for certain functionality, like
* [WebViewClient.shouldOverrideUrlLoading] to work properly.
*/
Comment on lines +7 to +12

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

internal class StripeWebChromeClient : WebChromeClient()
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.stripe.android.connect.webview

import android.content.Context
import android.net.Uri
import android.webkit.WebResourceRequest
import android.webkit.WebSettings
import android.webkit.WebView
import com.stripe.android.connect.EmbeddedComponentManager
Expand All @@ -16,17 +19,20 @@ import org.mockito.kotlin.doReturn
import org.mockito.kotlin.isNull
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import kotlin.test.assertFalse

@OptIn(PrivateBetaConnectSDK::class)
class StripeConnectWebViewClientTest {

private val mockContext: Context = mock()
private val mockSettings: WebSettings = mock {
on { userAgentString } doReturn "user-agent"
}
private val mockWebView: WebView = mock {
on { settings } doReturn mockSettings
on { context } doReturn mock()
on { context } doReturn mockContext
}
private val mockStripeIntentLauncher: StripeIntentLauncher = mock()

private lateinit var webViewClient: StripeConnectWebViewClient

Expand All @@ -40,6 +46,7 @@ class StripeConnectWebViewClientTest {
webViewClient = StripeConnectWebViewClient(
embeddedComponentManager = embeddedComponentManager,
connectComponent = StripeEmbeddedComponent.PAYOUTS,
stripeIntentLauncher = mockStripeIntentLauncher,
logger = Logger.getInstance(enableLogging = false),
jsonSerializer = Json { ignoreUnknownKeys = true },
)
Expand Down Expand Up @@ -67,4 +74,57 @@ class StripeConnectWebViewClientTest {
"https://connect-js.stripe.com/v1.0/android_webview.html#component=payouts&publicKey=pk_test_123"
)
}

@Test
fun `shouldOverrideUrlLoading doesn't handle request when view or request is null, returns false`() {
val resultNullView = webViewClient.shouldOverrideUrlLoading(view = null, request = mock())
assertFalse(resultNullView)

val resultNullRequest = webViewClient.shouldOverrideUrlLoading(view = mockWebView, request = null)
assertFalse(resultNullRequest)
}

@Test
fun `shouldOverrideUrlLoading allows request and returns false for allowlisted hosts`() {
val uri = mockUri("https", "connect-js.stripe.com", "/allowlisted")
val mockRequest = mock<WebResourceRequest> {
on { url } doReturn uri
}

val result = webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest)
assertFalse(result)
}

@Test
fun `shouldOverrideUrlLoading launches ChromeCustomTab for https urls`() {
val mockUri = mockUri("https", "example.com", "/test")
val mockRequest = mock<WebResourceRequest> {
on { url } doReturn mockUri
}

val result = webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest)

assert(result)
verify(mockStripeIntentLauncher).launchSecureExternalWebTab(mockContext, mockUri)
}

@Test
fun `shouldOverrideUrlLoading blocks url loading and returns true for unsupported schemes`() {
val mockUri = mockUri("mailto", "example.com", "/email")
val mockRequest = mock<WebResourceRequest> {
on { url } doReturn mockUri
}

val result = webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest)
assert(result)
}

private fun mockUri(scheme: String, host: String, path: String): Uri {
return mock<Uri> {
on { this.scheme } doReturn scheme
on { this.host } doReturn host
on { this.path } doReturn path
on { toString() } doReturn "$scheme://$host$path"
}
}
}
Loading