Merge pull request #21 from onelogin/copilot/fix-response-state-parameter

Add optional state parameter to OIDCConfiguration

Author: Subterrane

README.md

@@ -54,6 +54,7 @@ OIDCConfiguration.Builder()
     .scopes(listOf("openid"))
     .isDebug(true)
     .loginHint("test@email.com")
+    .state("custom-state-value")
     .build()
 ```
 
@@ -66,6 +67,7 @@ The supported parameters of the configuration are:
 | redirectUrl   | Redirect Url specified in the OneLogin Application                                                                  | Required |
 | scopes        | List of scopes of the authorization token, it should include `openid`                                               | Required |
 | loginHint     | A string hint to the Authorization Server about the login identifier the End-User might use to log in               | Optional |
+| state         | An opaque value used to maintain state between the request and callback to prevent CSRF attacks. If not provided, the library will auto-generate one | Optional |
 | isDebug       | Specifies if the instance of the library should be initialized in debug mode, which will log additional information | Optional |
 
 

oneloginoidc/src/main/java/com/onelogin/oidc/OIDCConfiguration.kt

@@ -8,6 +8,7 @@ class OIDCConfiguration private constructor(
     internal val redirectUrl: String,
     internal val scopes: List<String>,
     internal val loginHint: String?,
+    internal val state: String?,
     internal val encryptionManager: EncryptionManager?,
     internal val debug: Boolean
 ) {
@@ -18,6 +19,7 @@ class OIDCConfiguration private constructor(
         private var redirectUrl: String? = null
         private var scopes: List<String> = emptyList()
         private var loginHint: String? = null
+        private var state: String? = null
         private var encryptionManager: EncryptionManager? = null
         private var debug: Boolean = false
 
@@ -46,6 +48,11 @@ class OIDCConfiguration private constructor(
             return this
         }
 
+        fun state(state: String): Builder {
+            this.state = state
+            return this
+        }
+
         fun isDebug(debug: Boolean): Builder {
             this.debug = debug
             return this
@@ -79,6 +86,7 @@ class OIDCConfiguration private constructor(
                 redirectUrl!!,
                 scopes,
                 loginHint,
+                state,
                 encryptionManager,
                 debug
             )

oneloginoidc/src/main/java/com/onelogin/oidc/login/SignInManagerImpl.kt

@@ -100,6 +100,8 @@ internal class SignInManagerImpl(
 
         if (!configuration.loginHint.isNullOrBlank()) authReqBuilder.setLoginHint(configuration.loginHint)
 
+        if (!configuration.state.isNullOrBlank()) authReqBuilder.setState(configuration.state)
+
         return authReqBuilder.build()
     }
 }

oneloginoidc/src/test/java/com/onelogin/oidc/OIDCConfigurationTest.kt

@@ -73,4 +73,22 @@ class OIDCConfigurationTest {
         assertEquals(listOf("openid"), configuration.scopes)
         assertEquals("testHint", configuration.loginHint)
     }
+
+    @Test
+    @TestRail
+    fun testConfigurationWithStateParameter() {
+        val configuration = OIDCConfiguration.Builder()
+            .issuer("http://issuer.com")
+            .clientId("clientId")
+            .redirectUrl("redirectUrl")
+            .scopes(listOf("openid"))
+            .state("custom-state-123")
+            .build()
+
+        assertEquals("http://issuer.com", configuration.issuer)
+        assertEquals("clientId", configuration.clientId)
+        assertEquals("redirectUrl", configuration.redirectUrl)
+        assertEquals(listOf("openid"), configuration.scopes)
+        assertEquals("custom-state-123", configuration.state)
+    }
 }

oneloginoidc/src/test/java/com/onelogin/oidc/login/SignInManagerTest.kt

@@ -15,6 +15,7 @@ import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.channels.produce
 import kotlinx.coroutines.runBlocking
 import net.openid.appauth.*
+import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -186,4 +187,44 @@ class SignInManagerTest {
         verify { authorizationService.performTokenRequest(any(), any()) }
         verify { callback.onError(any()) }
     }
+
+    @Test
+    @TestRail
+    fun signInUsesCustomStateWhenProvided() = runBlocking {
+        val customState = "custom-state-value"
+        val configWithState = OIDCConfiguration.Builder()
+            .issuer("testIssuer")
+            .scopes(listOf("openid"))
+            .redirectUrl("redirectTest")
+            .clientId("testClientId")
+            .state(customState)
+            .build()
+
+        val signInManagerWithState = SignInManagerImpl(
+            configWithState,
+            authorizationService,
+            repository
+        ) { authRequest ->
+            // Verify the authorization request contains the custom state
+            assertEquals("Expected state to match custom state", customState, authRequest.state)
+            signInFragment
+        }
+
+        val serviceCallbackSlot = slot<AuthorizationService.TokenResponseCallback>()
+        every { signInFragment.resultChannel }.returns(produce<Pair<AuthorizationResponse?, AuthorizationException?>> {
+            send(spyResponse to null)
+        } as Channel)
+        every {
+            authorizationService.performTokenRequest(
+                any(),
+                capture(serviceCallbackSlot)
+            )
+        } answers {
+            serviceCallbackSlot.captured.onTokenRequestCompleted(mockk(), null)
+        }
+
+        signInManagerWithState.signIn(activity, callback)
+
+        verify { callback.onSuccess(any()) }
+    }
 }