Commit 97edf6ea authored by Grisha Kruglov's avatar Grisha Kruglov
Browse files

Closes #4350: Allow for a missing `action` param in FxA auth flows

It's possible that `action` parameter may be missing. This patch adds handling for that case,
and tests for the interceptor (which were missing entirely) that cover all combinations.
parent 3c03b079
......@@ -80,9 +80,9 @@ sealed class AuthType {
object Pairing : AuthType()
/**
* Account was created for an unknown external reason, identified by [action].
* Account was created for an unknown external reason, hopefully identified by [action].
*/
data class OtherExternal(val action: String) : AuthType()
data class OtherExternal(val action: String?) : AuthType()
/**
* Account created via a shared account state from another app.
......
......@@ -68,7 +68,7 @@ class FirefoxAccountsAuthFeature(
val code = parsedUri.getQueryParameter("code")
if (code != null) {
val authType = parsedUri.getQueryParameter("action")!!.toAuthType()
val authType = parsedUri.getQueryParameter("action").toAuthType()
val state = parsedUri.getQueryParameter("state") as String
// Notify the state machine about our success.
......
......@@ -21,9 +21,15 @@ import org.mockito.ArgumentMatchers.anyBoolean
import org.mockito.ArgumentMatchers.anyString
import org.mockito.Mockito.`when`
import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.concept.engine.request.RequestInterceptor
import mozilla.components.concept.sync.AuthFlowUrl
import mozilla.components.concept.sync.AuthType
import mozilla.components.service.fxa.DeviceConfig
import mozilla.components.service.fxa.FxaAuthData
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
// Same as the actual account manager, except we get to control how FirefoxAccountShaped instances
// are created. This is necessary because due to some build issues (native dependencies not available
......@@ -125,6 +131,78 @@ class FirefoxAccountsAuthFeatureTest {
assertEquals("https://accounts.firefox.com/signin", authLambda.url)
}
@Test
fun `auth interceptor`() {
val manager = mock<FxaAccountManager>()
val redirectUrl = "https://accounts.firefox.com/oauth/success/123"
val feature = FirefoxAccountsAuthFeature(
manager,
redirectUrl,
mock()
) { _, _ -> }
// Non-final FxA url.
assertNull(feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/not/the/right/url"))
verify(manager, never()).finishAuthenticationAsync(any())
// Non-FxA url.
assertNull(feature.interceptor.onLoadRequest(mock(), "https://www.wikipedia.org"))
verify(manager, never()).finishAuthenticationAsync(any())
// Redirect url, without code/state.
assertNull(feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/"))
verify(manager, never()).finishAuthenticationAsync(any())
// Redirect url, without code/state.
assertNull(feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/test"))
verify(manager, never()).finishAuthenticationAsync(any())
// Code+state, no action.
assertEquals(
RequestInterceptor.InterceptionResponse.Url(redirectUrl),
feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/?code=testCode1&state=testState1")
)
verify(manager).finishAuthenticationAsync(
FxaAuthData(authType = AuthType.OtherExternal(null), code = "testCode1", state = "testState1")
)
// Code+state, action=signin.
assertEquals(
RequestInterceptor.InterceptionResponse.Url(redirectUrl),
feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/?code=testCode2&state=testState2&action=signin")
)
verify(manager).finishAuthenticationAsync(
FxaAuthData(authType = AuthType.Signin, code = "testCode2", state = "testState2")
)
// Code+state, action=signup.
assertEquals(
RequestInterceptor.InterceptionResponse.Url(redirectUrl),
feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/?code=testCode3&state=testState3&action=signup")
)
verify(manager).finishAuthenticationAsync(
FxaAuthData(authType = AuthType.Signup, code = "testCode3", state = "testState3")
)
// Code+state, action=pairing.
assertEquals(
RequestInterceptor.InterceptionResponse.Url(redirectUrl),
feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/?code=testCode4&state=testState4&action=pairing")
)
verify(manager).finishAuthenticationAsync(
FxaAuthData(authType = AuthType.Pairing, code = "testCode4", state = "testState4")
)
// Code+state, action is an unknown value.
assertEquals(
RequestInterceptor.InterceptionResponse.Url(redirectUrl),
feature.interceptor.onLoadRequest(mock(), "https://accounts.firefox.com/oauth/success/123/?code=testCode5&state=testState5&action=someNewActionType")
)
verify(manager).finishAuthenticationAsync(
FxaAuthData(authType = AuthType.OtherExternal("someNewActionType"), code = "testCode5", state = "testState5")
)
}
private fun prepareAccountManagerForSuccessfulAuthentication(): TestableFxaAccountManager {
val mockAccount: OAuthAccount = mock()
val profile = Profile(uid = "testUID", avatar = null, email = "test@example.com", displayName = "test profile")
......
......@@ -23,12 +23,13 @@ import mozilla.components.concept.sync.SyncAuthInfo
* Converts a raw 'action' string into an [AuthType] instance.
* Actions come to us from FxA during an OAuth login, either over the WebChannel or via the redirect URL.
*/
fun String.toAuthType(): AuthType {
fun String?.toAuthType(): AuthType {
return when (this) {
"signin" -> AuthType.Signin
"signup" -> AuthType.Signup
"pairing" -> AuthType.Pairing
// We want to gracefully handle 'actions' we don't know about.
// We want to gracefully handle an 'action' we don't know about.
// This also covers the `null` case.
else -> AuthType.OtherExternal(this)
}
}
......
......@@ -75,6 +75,9 @@ permalink: /changelog/
* **support-utils**
* `Intent.asForegroundServicePendingIntent(Context)` extension method to create pending intent for the service that will play nicely with background execution limitations introduced in Android O (e.g. foreground service).
* **concept-sync**
* ⚠️ **This is a breaking change**: `action` param of `AuthType.OtherExternal` is now optional. Missing `action` indicates that we really don't know what external authType we've hit.
# 11.0.0
* [Commits](https://github.com/mozilla-mobile/android-components/compare/v10.0.0...v11.0.0)
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment