Automate authenticator oauth tests #432

hud10837 · 2024-05-16T13:02:14Z

Adds tests for OAuth sign in. This covers the manual test cases 1 and 2. Like the ServerTrustTests, the test logic is generally implemented in a single util function that is called by each test along with the corresponding test action (eg. enter credentials and sign in)

I found that because we are calling handleArcGISAuthenticationChallenge directly, the tests were catching any OperationCancelledExceptions that were thrown by that method. It seems like this was an oversight so I have changed the implementation and it seems to be working (at least, the tests are passing)

Currently the signIn tests will fail because they don't have working credentials until we come up with a solution for test credentials in the toolkit.

This also uses the microapp's oauth configuration as well as the sample repo's oauth configuration in order to avoid a launcher disambiguation dialog in the tests. This is something we should confirm with @shubham7109 is okay or we should create new redirect urls and clientIds.

The tests use two new OAuth clientId/redirectUrls.

…ond correctly to cancellation errors

…lkit into hud10837/automate-authenticator-oauth-tests

hud10837 · 2024-05-16T13:03:35Z

toolkit/authentication/build.gradle.kts

+    packagingOptions {
+        exclude("META-INF/LICENSE-notice.md")
+        exclude("META-INF/LICENSE.md")
+    }


I couldn't compile after adding uiautomator and mockk without this, but I'm not sure if that's just a local issue

I think that change is correct.

hud10837 · 2024-05-16T13:04:07Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

Test Case 1

hud10837 · 2024-05-16T13:04:22Z

...ication/src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthUserLauncherTests.kt

Test Case 2

hud10837 · 2024-05-16T13:06:43Z

...kit/authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticatorState.kt

+                    }.getOrElse {
+                        if (it is OperationCancelledException) {
+                            return ArcGISAuthenticationChallengeResponse.Cancel
+                        } else {
+                            return ArcGISAuthenticationChallengeResponse.ContinueAndFailWithError(it)
+                        }
+                    }


We should discuss this. Because the tests use handleArcGISAuthenticationChallenge() directly, any exceptions that get thrown are thrown into the test. Normally the caller of handleArcGISAuthenticationChallenge will handle the exceptions but it seems to me that we should be handling it here anyway.

I don't think this change is needed. Any exception thrown, would be handled in the SDK's AuthenticationChallengeQueue. In the test code, since we bypass that code, we should just catch the exception there.

gunt0001

@hud10837 looks good, a few comments.

gunt0001 · 2024-05-17T14:34:35Z

toolkit/authentication/src/androidTest/AndroidManifest.xml

+
+                <data
+                    android:host="auth"
+                    android:scheme="kotlin-authentication-test-1" />


gunt0001 · 2024-05-17T14:36:51Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+    @Before
+    fun signOut() {
+        runBlocking {
+            ArcGISEnvironment.authenticationManager.signOut()


Call sign out also in @After?

gunt0001 · 2024-05-17T14:38:08Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+    @After
+    fun closeBrowser() {
+        try {
+            UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()).pressBack()


Is this required? As long as the test uses a CCT, it should close automatically at the end of each test?

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+            enterCredentialsOnBrowser("username", "password", composeTestRule.activity)
+            clickByText("Sign In")
+        }.await()
+        assert(response is ArcGISAuthenticationChallengeResponse.ContinueWithCredential)


gunt0001 · 2024-05-17T14:43:48Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+    val composeTestRule = createAndroidComposeRule<ComponentActivity>()
+
+    private val arcGISOnline = "https://arcgis.com"
+    private val authenticatorState = AuthenticatorState().apply {


I think this state object should be created locally within the scope of testOAuthChallengeWithStateRestoration(), so that a new Authenticator() call is passed a new instance of AuthenticatorState.

gunt0001 · 2024-05-17T14:50:01Z

...ication/src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthUserLauncherTests.kt

+    @After
+    fun closeBrowser() {
+        try {
+            UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()).pressBack()


If we don't do this, does this cause any issues?

...ication/src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthUserLauncherTests.kt

+    @Before
+    fun signOut() {
+        runBlocking {
+            ArcGISEnvironment.authenticationManager.signOut()


...ication/src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthUserLauncherTests.kt

+            enterCredentialsOnBrowser("username", "password", composeTestRule.activity)
+            clickByText("Sign In")
+        }.await()
+        assert(response is ArcGISAuthenticationChallengeResponse.ContinueWithCredential)


gunt0001 · 2024-05-17T14:56:47Z

toolkit/authentication/src/main/AndroidManifest.xml

@@ -18,5 +18,6 @@
  -->

 <manifest xmlns:android="http://schemas.android.com/apk/res/android">
+    <uses-permission android:name="android.permission.INTERNET" />


The toolkit should not set any permissions in its manifest. This is the responsibility of the user.

gunt0001 · 2024-05-17T15:06:20Z

...kit/authentication/src/main/java/com/arcgismaps/toolkit/authentication/AuthenticatorState.kt

+                    }.getOrElse {
+                        if (it is OperationCancelledException) {
+                            return ArcGISAuthenticationChallengeResponse.Cancel
+                        } else {
+                            return ArcGISAuthenticationChallengeResponse.ContinueAndFailWithError(it)
+                        }
+                    }


I don't think this change is needed. Any exception thrown, would be handled in the SDK's AuthenticationChallengeQueue. In the test code, since we bypass that code, we should just catch the exception there.

hud10837 · 2024-05-21T14:22:25Z

Thanks @gunt0001 I believe I have addressed all of your comments

…uration tests

gunt0001 · 2024-05-23T13:00:11Z

toolkit/authentication/build.gradle.kts

+    packagingOptions {
+        exclude("META-INF/LICENSE-notice.md")
+        exclude("META-INF/LICENSE.md")
+    }


I think that change is correct.

gunt0001 · 2024-05-23T13:02:48Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+    @Test
+    fun successfulSignIn() = runTest {
+        val response = testOAuthChallengeWithStateRestoration {
+            InstrumentationRegistry.getInstrumentation().context.startActivity(getSuccessfulRedirectIntent("kotlin-authentication-test-1://auth"))


Could you add a comment here, something along the lines of...

Suggested change

InstrumentationRegistry.getInstrumentation().context.startActivity(getSuccessfulRedirectIntent("kotlin-authentication-test-1://auth"))

// When the OAuth sign in dialog shows up, we simulate successful sign in by launching an intent with the redirect URl, which otherwise would be sent from the Portal server, but would require valid credentials

InstrumentationRegistry.getInstrumentation().context.startActivity(getSuccessfulRedirectIntent("kotlin-authentication-test-1://auth"))

gunt0001 · 2024-05-23T13:07:41Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+    ): Deferred<Result<ArcGISAuthenticationChallengeResponse>> {
+        ArcGISEnvironment.configureArcGISHttpClient {
+            interceptTokenRequests()
+        }


I suggest to move this part of setting up an interceptor out of this function and call this individually as part of each test function. I don't think it belongs into this function here.

gunt0001 · 2024-05-23T13:08:41Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+            oAuthUserConfiguration = OAuthUserConfiguration(
+                "https://arcgis.com",
+                // This client ID is for demo purposes only. For use of the Authenticator in your own app,
+                //            // create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/


Comment needs formating

Suggested change

// // create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/

// create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/

gunt0001 · 2024-05-23T13:09:03Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+                modifier = Modifier.testTag("Authenticator")
+            )
+        }
+        // isse the OAuth challenge


Suggested change

// isse the OAuth challenge

// issue the OAuth challenge

gunt0001 · 2024-05-23T13:19:04Z

...cation/src/androidTest/java/com/arcgismaps/toolkit/authentication/TestAuthenticationUtils.kt

+ *
+ * @since 200.5.0
+ */
+fun ArcGISHttpClient.Builder.interceptTokenRequests() {


nitpick naming suggestion:

Suggested change

fun ArcGISHttpClient.Builder.interceptTokenRequests() {

fun ArcGISHttpClient.Builder.setupTokenRequestInterceptor() {

gunt0001 · 2024-05-23T13:19:30Z

...cation/src/androidTest/java/com/arcgismaps/toolkit/authentication/TestAuthenticationUtils.kt

+ *
+ * @since 200.5.0
+ */
+fun Request.getFakeTokenResponse(): Response =


nitpick naming suggestion:

Suggested change

fun Request.getFakeTokenResponse(): Response =

fun Request.createFakeTokenResponse(): Response =

gunt0001 · 2024-05-23T13:19:44Z

...cation/src/androidTest/java/com/arcgismaps/toolkit/authentication/TestAuthenticationUtils.kt

+        addHeader("transfer-encoding", "chunked")
+        addHeader("vary", "X-Esri-Authorization")
+        addHeader("x-content-type-options", "nosniff")
+    }.build()


missing newline

gunt0001 · 2024-05-23T13:19:55Z

...hentication/src/androidTest/java/com/arcgismaps/toolkit/authentication/UiDeviceExtensions.kt

@@ -0,0 +1,80 @@
+package com.arcgismaps.toolkit.authentication


missing copyright info

gunt0001 · 2024-05-23T13:20:50Z

...hentication/src/androidTest/java/com/arcgismaps/toolkit/authentication/UiDeviceExtensions.kt

+ * found.
+ *
+ * @param packageId the view's package Id to wait for.
+ * @since 200.4.0


Needs updating also in other places in this file...

Suggested change

* @since 200.4.0

* @since 200.5.0

gunt0001

Approving so this can move to second review once comments have been addressed.

alan-edi

@hud10837 Looks good, just a few minor comments to consider

alan-edi · 2024-05-28T09:23:12Z

...src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthDefaultConfigurationTests.kt

+            oAuthUserConfiguration = OAuthUserConfiguration(
+                "https://arcgis.com",
+                // This client ID is for demo purposes only. For use of the Authenticator in your own app,
+                // create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/


Newline before the URL, to avoid line being unnecessarily long?

alan-edi · 2024-05-28T10:28:59Z

...ication/src/androidTest/java/com/arcgismaps/toolkit/authentication/OAuthUserLauncherTests.kt

+        oAuthUserConfiguration = OAuthUserConfiguration(
+            "https://arcgis.com/",
+            // This client ID is for demo purposes only. For use of the Authenticator in your own app,
+            // create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/


Newline before the URL?

alan-edi · 2024-05-28T10:52:04Z

...hentication/src/androidTest/java/com/arcgismaps/toolkit/authentication/UiDeviceExtensions.kt

+
+
+/**
+ * Waits for the View with the matching package to be visible. Throws an error if the view can't be


package -> [packageId] and delete the @param below?

alan-edi · 2024-05-28T11:01:10Z

...hentication/src/androidTest/java/com/arcgismaps/toolkit/authentication/UiDeviceExtensions.kt

+ *
+ * @since 200.5.0
+ */
+fun UiDevice.enterCredentialsOnBrowser(username: String, password: String, activity: Activity) {


Seems this function, and the functions below that it calls, isn't used, but I guess you expect to use it later when we come up with a solution for test credentials in the toolkit?

I will remove them for now, can always return to this PR to copy them.

alan-edi · 2024-05-28T11:21:01Z

Oh, one other thing - there are no tests for the following sub-cases, but I guess you have good reasons:

1.2 Incorrect Credentials - maybe do it later when we come up with a solution for test credentials in the toolkit?
1.5 Pressing the Close Button - is it practicable to automate that?
1.6 Press the Home Button - again, is it practicable?

hud10837 · 2024-05-29T06:17:19Z

@alan-edi thanks for the review. Regarding those test cases:

1.2 Incorrect Credentials - I'm not sure there's any value in testing this (also true for the manual tests) because there is no interaction with the Authenticator when you enter invalid credentials -- everything continues to happen in the browser, and if you enter the wrong credentials too many times, it keeps the browser open and locks you out for 15 minutes, so there is no "invalid credentials" redirect back to the app.
1.5 Pressing the Close Button - It's possible to automate this, but a bit difficult, and the behavior should be identical to the back button. But I am happy to add this test case if you think it's a good idea
1.6 Press the Home Button - The goal of this in the manual tests was to test the activity being destroyed and put in the background, then recreated, which also happens during configuration changes. I think this is very difficult to automate though, so I think that testing only device rotation is sufficient (as it should be the same behavior anyway) What do you think?

alan-edi · 2024-05-29T08:41:44Z

@hud10837 I'm happy with what you say re test cases 1.2, 1.5 and 1.6, thanks

hud10837 · 2024-05-29T08:57:59Z

Thanks @alan-edi @gunt0001 !

hud10837 added 14 commits May 8, 2024 16:43

backup

68aa706

use advanceUntilIdle

5352863

implement basic oauth sign in test

21cd4e4

add remaining test cases

63b56cf

update authenticatorState.handleArcGISAuthenticationChallenge to resp…

bbfebc9

…ond correctly to cancellation errors

use RestorationStateTester

5ceb9ee

use device rotation instead of staterestorationtester

5f9e025

create test case 2 file

aae6d26

Add remaining OAuthUserLauncherTests

f3b58c8

extract util funs to file

d03e641

Merge branch 'v.next' of github.com:ArcGIS/arcgis-maps-sdk-kotlin-too…

50d32a3

…lkit into hud10837/automate-authenticator-oauth-tests

use samples redirect url

8fcc3bd

add more doc

ac00c3b

update gradle

07a2201

hud10837 commented May 16, 2024

View reviewed changes

fix unexpected changes

85c7714

hud10837 marked this pull request as ready for review May 16, 2024 13:12

update redirect urls and client ids

5442688

gunt0001 assigned hud10837 May 17, 2024

gunt0001 self-requested a review May 17, 2024 14:32

gunt0001 reviewed May 17, 2024

View reviewed changes

hud10837 added 5 commits May 21, 2024 13:05

signout after

5859aaf

assert is OAuth credential

1ff9b7b

mv authenticatorState to local val

c9c3d7a

don't declare permission in manifest

2e94d8e

revert changes to AuthenticatorState and use Result type for tests

056517b

hud10837 requested a review from gunt0001 May 21, 2024 14:22

hud10837 added 2 commits May 23, 2024 09:28

perform successful signin with faked token response on Default Config…

65b5647

…uration tests

use interceptor strategy on user launcher tests

5316986

hud10837 added 2 commits May 23, 2024 09:41

use 200.5.0

e26ad16

rename funs and fix doc

40e6ef4

gunt0001 reviewed May 23, 2024

View reviewed changes

gunt0001 approved these changes May 23, 2024

View reviewed changes

hud10837 added 4 commits May 27, 2024 11:07

add explanatory comment

076f5a6

mv token interceptor configuration to individual tests

3952c4f

fix formatting in various places

04e8c7f

rename util funs

bf2a28e

alan-edi self-assigned this May 28, 2024

alan-edi approved these changes May 28, 2024

View reviewed changes

hud10837 added 2 commits May 29, 2024 08:53

fix doc

52c6bcc

rm unused util funs

2e07393

alan-edi removed their assignment May 29, 2024

hud10837 merged commit 0de21d1 into v.next May 29, 2024

hud10837 deleted the hud10837/automate-authenticator-oauth-tests branch May 29, 2024 08:58

	InstrumentationRegistry.getInstrumentation().context.startActivity(getSuccessfulRedirectIntent("kotlin-authentication-test-1://auth"))
	// When the OAuth sign in dialog shows up, we simulate successful sign in by launching an intent with the redirect URl, which otherwise would be sent from the Portal server, but would require valid credentials
	InstrumentationRegistry.getInstrumentation().context.startActivity(getSuccessfulRedirectIntent("kotlin-authentication-test-1://auth"))

	// // create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/
	// create your own client ID. For more info see: https://developers.arcgis.com/documentation/mapping-apis-and-services/security/tutorials/register-your-application/

	fun ArcGISHttpClient.Builder.interceptTokenRequests() {
	fun ArcGISHttpClient.Builder.setupTokenRequestInterceptor() {

	fun Request.getFakeTokenResponse(): Response =
	fun Request.createFakeTokenResponse(): Response =

		@@ -0,0 +1,80 @@
		package com.arcgismaps.toolkit.authentication



		/**
		* Waits for the View with the matching package to be visible. Throws an error if the view can't be

Automate authenticator oauth tests #432

Automate authenticator oauth tests #432

Conversation

hud10837 commented May 16, 2024 • edited

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

gunt0001 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

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hud10837 commented May 21, 2024

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

gunt0001 left a comment

Choose a reason for hiding this comment

alan-edi 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

alan-edi commented May 28, 2024

hud10837 commented May 29, 2024

alan-edi commented May 29, 2024

hud10837 commented May 29, 2024

hud10837 commented May 16, 2024 •

edited