Fail gracefully when the default browser does not support custom tabs #1039

eri9000 · 2025-12-06T00:13:10Z

Related to issue: https://devtopia.esri.com/runtime/kotlin/issues/6659

Summary of changes:

Adds test to verify AuthenticationActivity behavior
Modifies AuthenticationActivity to process incoming redirects in through onCreate.
Provides better error handling when
Modifies OAuthUserSignInContract to handle errors coming back in the Result
Creates a new Authenticator Exceptions class

Pre-merge Checklist

a vTest Job for this PR has been run
- link:
Unit and/or integration tests have been added to exercise this PR's logic, and the tests are passing:
- Yes
- No

…rowser is launched

…and simplify intent management

…dles it

…lt browser

gunt0001 · 2025-12-12T16:19:02Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+    /**
+     * Given [AuthenticationActivity] with a browser that supports Custom Tabs
+     * When [AuthenticationActivity] starts
+     * Then an intent should be fired with ACTION_VIEW, simulating a Custom Tabs launch


I'm confused - looking at processAuthenticationFlow(), when an intent is handled with ACTION_VIEW, that means we are getting the redirect from Custom Tabs. It doesn't mean we are launching Custom Tabs. If this test is meant to check that the custom tabsl intent is being launched, shouldn't we be checking for something like the intent being of type CustomTabsIntent?

These are the following intents that are captured by the test,

AuthenticationActivityTest: Intent: action=null data=null extras=KEY_INTENT_EXTRA_URL AuthenticationActivityTest: Intent: action=android.intent.action.VIEW data=https://example.com/auth extras=android.support.customtabs.extra.SESSION, android.support.customtabs.extra.EXTRA_ENABLE_INSTANT_APPS, androidx.browser.customtabs.extra.SHARE_STATE, com.android.browser.headers

As you can see, the first one is the one that the test uses to launch AuthenticationActivity, simulating the OAuthUserSignInContract.

And the second one is the CustomTabs intent that AuthenticationActivity launches.
Since we are not signing in, we do not see the "redirect" intent that we would get from the browser

If this test is meant to check that the custom tabs intent is being launched, shouldn't we be checking for something like the intent being of type CustomTabsIntent?

I assume you mean some identifiable information that it will tell us that it is the CustomTabs Intent, I already check for android.support.customtabs.extra.SESSION inside the test but I could add more if you think is necessary

gunt0001 · 2025-12-12T16:23:05Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+        scenario.close()
+        val result = scenario.result
+        assertThat(result.resultCode).isEqualTo(RESULT_CODE_SUCCESS)
+        assertThat(result.resultData?.getStringExtra("KEY_INTENT_EXTRA_RESPONSE_URI")).isEqualTo(redirectUri)


How is this test working? In other words, what closes the Custom Tabs and redirects into AuthenticationActivity with RESULT_CODE_SUCCESS?

This test does not open CustomTabs, it only verifies that AuthenticationActivity handles a "redirect" URI successfully.
It does this by launching the activity with an intent that has a redirect_uri as data, simulating a browser redirecting back to the activity.

When AuthenticationActivity processes a redirect URI successfully, it calls setResult(RESULT_CODE_SUCCESS, newIntent) here

Ok, makes sense. Would the following doc make that clearer?

/** * Given [AuthenticationActivity] receives an intent containing a valid redirect URI * When the activity processes the redirect intent * Then the activity finishes with RESULT_CODE_SUCCESS and includes the redirect URI in the result data * @since 300.0.0 */

gunt0001 · 2025-12-12T16:24:29Z

...authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticationActivity.kt

+     *   an appropriate error message.
+     * @since 300.0.0
+     */
+    private fun processAuthenticationFlow() {


Nitpick - naming suggestion:

Suggested change

private fun processAuthenticationFlow() {

private fun promptWithCustomTabs() {

i'd like to give it a more generic name since this likely will be the method to launch external browser or Auth Tabs (when we support it).

I thought we won't support external browser? And when we move to Auth Tabs, we can rename the function to promptWithAuthTab at that point.

...authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticationActivity.kt

gunt0001 · 2025-12-12T16:33:26Z

...authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticationActivity.kt

+            val authEndpoint = getStringExtra(KEY_INTENT_EXTRA_URL)
+            when {
+                // This signals that we have been redirected back to the app from the browser
+                action == Intent.ACTION_VIEW && data != null -> {


Shouldn't this case only happening when we get an intent in onNewIntent? It seems we ar enow conflating tow different cases in processAuthenticationFlow:

launching custom tabs

handling the redirect from custom tabs

I think this function here should only handle the first case.

When testing different browsers sign-in, i found that some browsers might redirect with different flags and might come through onCreate instead of onNewIntent.
This is just safe guarding this possible outcome.

This is diverging quite a bit from the current implementation. At the minimum we will need test cases for the scenarios you mention. Lets discuss...

eri9000 · 2025-12-12T23:43:12Z

@gunt0001 Please have another look

gunt0001 · 2025-12-16T15:51:24Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+ * @since 300.0.0
+ */
+@RunWith(AndroidJUnit4::class)
+class AuthenticationActivityExternalBrowserTest {


Could we name it simply?

Suggested change

class AuthenticationActivityExternalBrowserTest {

class AuthenticationActivityTest {

gunt0001 · 2025-12-16T16:06:47Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+    /**
+     * Given [AuthenticationActivity] is launched with a default browser that does not support Custom Tabs
+     * When [AuthenticationActivity] starts
+     * Then the activity finishes with RESULT_CODE_CANCELED and includes an exception message in the result data
+     * @since 300.0.0
+     */


Would the following be a better description for this test?

Suggested change

/**

* Given [AuthenticationActivity] is launched with a default browser that does not support Custom Tabs

* When [AuthenticationActivity] starts

* Then the activity finishes with RESULT_CODE_CANCELED and includes an exception message in the result data

* @since 300.0.0

*/

/**

* Given a device with default browser that does not support Custom Tabs

* When [AuthenticationActivity] receives an intent for OAuth sign-in

* Then the activity finishes with RESULT_CODE_CANCELED and includes an exception message in the result data

* @since 300.0.0

*/

gunt0001 · 2025-12-16T16:12:59Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+
+        // Launch the AuthenticationActivity with the redirect intent
+        val scenario = ActivityScenario.launchActivityForResult<AuthenticationActivity>(intent)
+        scenario.close()


Looking at the doc of close, should we be using scenario.use { ... } here or alternatively ActivityScenarioRule? The same would apply to the other tests.

gunt0001 · 2025-12-16T16:15:22Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+        scenario.close()
+        val result = scenario.result
+        assertThat(result.resultCode).isEqualTo(RESULT_CODE_SUCCESS)
+        assertThat(result.resultData?.getStringExtra("KEY_INTENT_EXTRA_RESPONSE_URI")).isEqualTo(redirectUri)


Ok, makes sense. Would the following doc make that clearer?

/** * Given [AuthenticationActivity] receives an intent containing a valid redirect URI * When the activity processes the redirect intent * Then the activity finishes with RESULT_CODE_SUCCESS and includes the redirect URI in the result data * @since 300.0.0 */

gunt0001 · 2025-12-16T16:17:25Z

...Test/java/com/arcgismaps/toolkit/authentication/AuthenticationActivityExternalBrowserTest.kt

+     * @since 300.0.0
+     */
+    @Test
+    fun returnsExceptionWhenNoBrowserAvailable() {


Should this be named?

Suggested change

fun returnsExceptionWhenNoBrowserAvailable() {

fun returnsExceptionWhenNoCustomTabsAvailable() {

gunt0001 · 2025-12-16T16:19:02Z

...authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticationActivity.kt

+     *   an appropriate error message.
+     * @since 300.0.0
+     */
+    private fun processAuthenticationFlow() {


I thought we won't support external browser? And when we move to Auth Tabs, we can rename the function to promptWithAuthTab at that point.

gunt0001 · 2025-12-16T16:20:21Z

...authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticationActivity.kt

+            val authEndpoint = getStringExtra(KEY_INTENT_EXTRA_URL)
+            when {
+                // This signals that we have been redirected back to the app from the browser
+                action == Intent.ACTION_VIEW && data != null -> {


This is diverging quite a bit from the current implementation. At the minimum we will need test cases for the scenarios you mention. Lets discuss...

eri9000 · 2025-12-16T23:50:27Z

a simpler implementation is in PR #1044 since we want to make gradual changes

eri9000 added 14 commits November 26, 2025 14:19

Adds fallback to launch authentication in external browser

721e1c5

adds additional flag

b322d02

adds espresso test

2cc7ac8

adds test to verify AuthenticationActivity's behavior when external b…

e8e49ac

…rowser is launched

refactor AuthenticationActivity to improve redirect URI handling

6eb01a9

refactor AuthenticationActivity to simplify redirect URI handling

7fcd835

remove unused imports and logging in Extensions.kt

1bf954a

refactor AuthenticationActivity to enhance external browser handling …

cd36b4b

…and simplify intent management

adds flag to allow external browser to AuthenticationActivity and han…

e3b4969

…dles it

adds tests to verify behavior

7108bf0

adds a few TODOs and cleans up test

1da753c

Adds implementation to fail when custom tabs isn't supported by defau…

236fd9f

…lt browser

allow graceful cancellation from external app launch

0cc06e8

cleanup

9da8a74

eri9000 changed the base branch from main to v.next December 6, 2025 00:13

update else statement

4816c22

eri9000 changed the title ~~Erick/fail when custom tabs not found~~ Fail gracefully when the default browser does not support custom tabs Dec 6, 2025

updates test

92d30d2

eri9000 mentioned this pull request Dec 8, 2025

Launch OAuth in an external browser #1038

Draft

3 tasks

eri9000 added 4 commits December 8, 2025 11:12

adds CustomTabsNotFoundException

428bb6d

throws exception when invoking launchCustomTabs

de899cb

updates handling of CustomTabsNotFoundException

4d51976

code cleanup

166430a

eri9000 requested a review from gunt0001 December 8, 2025 22:16

eri9000 marked this pull request as ready for review December 8, 2025 22:16

removes mistake

59bc1f9

gunt0001 assigned eri9000 Dec 12, 2025

gunt0001 reviewed Dec 12, 2025

View reviewed changes

eri9000 requested a review from gunt0001 December 12, 2025 23:38

Adds more comments to test

fe01874

gunt0001 reviewed Dec 16, 2025

View reviewed changes

eri9000 closed this Dec 16, 2025

	private fun processAuthenticationFlow() {
	private fun promptWithCustomTabs() {

	class AuthenticationActivityExternalBrowserTest {
	class AuthenticationActivityTest {

	fun returnsExceptionWhenNoBrowserAvailable() {
	fun returnsExceptionWhenNoCustomTabsAvailable() {

Fail gracefully when the default browser does not support custom tabs #1039

Fail gracefully when the default browser does not support custom tabs #1039

Uh oh!

Conversation

eri9000 commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes:

Pre-merge Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eri9000 Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eri9000 Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eri9000 commented Dec 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eri9000 commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eri9000 commented Dec 6, 2025 •

edited

Loading

eri9000 Dec 12, 2025 •

edited

Loading

eri9000 Dec 12, 2025 •

edited

Loading