Merge pull request #7026 from thundernest/refactor_oauth_flow
Refactor OAuthFlow
This commit is contained in:
commit
a5d14de34a
10 changed files with 206 additions and 51 deletions
|
@ -1,6 +1,7 @@
|
|||
package com.fsck.k9
|
||||
|
||||
import app.k9mail.autodiscovery.providersxml.autodiscoveryProvidersXmlModule
|
||||
import app.k9mail.feature.account.oauth.featureAccountOAuthModule
|
||||
import com.fsck.k9.account.accountModule
|
||||
import com.fsck.k9.activity.activityModule
|
||||
import com.fsck.k9.contacts.contactsModule
|
||||
|
@ -21,6 +22,7 @@ import com.fsck.k9.ui.uiModule
|
|||
import com.fsck.k9.view.viewModule
|
||||
|
||||
val uiModules = listOf(
|
||||
featureAccountOAuthModule,
|
||||
uiBaseModule,
|
||||
activityModule,
|
||||
uiModule,
|
||||
|
|
|
@ -1,12 +1,16 @@
|
|||
package com.fsck.k9.activity
|
||||
|
||||
import app.k9mail.feature.account.oauth.domain.usecase.SuggestServerName
|
||||
import com.fsck.k9.activity.setup.AuthViewModel
|
||||
import org.koin.androidx.viewmodel.dsl.viewModel
|
||||
import org.koin.dsl.module
|
||||
|
||||
val activityModule = module {
|
||||
single { MessageLoaderHelperFactory(messageViewInfoExtractorFactory = get(), htmlSettingsProvider = get()) }
|
||||
factory { SuggestServerName() }
|
||||
viewModel { AuthViewModel(application = get(), accountManager = get(), oAuthConfigurationProvider = get()) }
|
||||
viewModel {
|
||||
AuthViewModel(
|
||||
application = get(),
|
||||
accountManager = get(),
|
||||
getOAuthRequestIntent = get(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
@ -5,7 +5,7 @@ import android.content.Intent
|
|||
import android.os.Bundle
|
||||
import android.view.View
|
||||
import app.k9mail.core.common.mail.Protocols
|
||||
import app.k9mail.feature.account.oauth.domain.usecase.SuggestServerName
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.SuggestServerName
|
||||
import com.fsck.k9.Account
|
||||
import com.fsck.k9.Preferences
|
||||
import com.fsck.k9.helper.EmailHelper.getDomainFromEmailAddress
|
||||
|
|
|
@ -8,14 +8,13 @@ import android.content.Intent
|
|||
import androidx.activity.result.ActivityResultLauncher
|
||||
import androidx.activity.result.ActivityResultRegistry
|
||||
import androidx.activity.result.contract.ActivityResultContract
|
||||
import androidx.core.net.toUri
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.DefaultLifecycleObserver
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import app.k9mail.core.common.oauth.OAuthConfiguration
|
||||
import app.k9mail.core.common.oauth.OAuthConfigurationProvider
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.GetOAuthRequestIntent
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.GetOAuthRequestIntent.GetOAuthRequestIntentResult
|
||||
import com.fsck.k9.Account
|
||||
import com.fsck.k9.preferences.AccountManager
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
|
@ -27,11 +26,8 @@ import kotlinx.coroutines.launch
|
|||
import kotlinx.coroutines.withContext
|
||||
import net.openid.appauth.AuthState
|
||||
import net.openid.appauth.AuthorizationException
|
||||
import net.openid.appauth.AuthorizationRequest
|
||||
import net.openid.appauth.AuthorizationResponse
|
||||
import net.openid.appauth.AuthorizationService
|
||||
import net.openid.appauth.AuthorizationServiceConfiguration
|
||||
import net.openid.appauth.ResponseTypeValues
|
||||
import timber.log.Timber
|
||||
|
||||
private const val KEY_AUTHORIZATION = "app.k9mail_auth"
|
||||
|
@ -39,7 +35,7 @@ private const val KEY_AUTHORIZATION = "app.k9mail_auth"
|
|||
class AuthViewModel(
|
||||
application: Application,
|
||||
private val accountManager: AccountManager,
|
||||
private val oAuthConfigurationProvider: OAuthConfigurationProvider,
|
||||
private val getOAuthRequestIntent: GetOAuthRequestIntent,
|
||||
) : AndroidViewModel(application) {
|
||||
private var authService: AuthorizationService? = null
|
||||
private val authState = AuthState()
|
||||
|
@ -88,54 +84,26 @@ class AuthViewModel(
|
|||
val account = checkNotNull(account)
|
||||
|
||||
viewModelScope.launch {
|
||||
val config = findOAuthConfiguration(account)
|
||||
if (config == null) {
|
||||
_uiState.update { AuthFlowState.NotSupported }
|
||||
return@launch
|
||||
}
|
||||
|
||||
try {
|
||||
startLogin(account, config)
|
||||
startLogin(account)
|
||||
} catch (e: ActivityNotFoundException) {
|
||||
_uiState.update { AuthFlowState.BrowserNotFound }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun startLogin(account: Account, config: OAuthConfiguration) {
|
||||
val authRequestIntent = withContext(Dispatchers.IO) {
|
||||
createAuthorizationRequestIntent(account.email, config)
|
||||
private suspend fun startLogin(account: Account) {
|
||||
val authRequestIntentResult = withContext(Dispatchers.IO) {
|
||||
getOAuthRequestIntent.execute(account.incomingServerSettings.host!!, account.email)
|
||||
}
|
||||
|
||||
resultObserver.login(authRequestIntent)
|
||||
}
|
||||
when (authRequestIntentResult) {
|
||||
GetOAuthRequestIntentResult.NotSupported -> {
|
||||
_uiState.update { AuthFlowState.NotSupported }
|
||||
}
|
||||
|
||||
private fun createAuthorizationRequestIntent(email: String, config: OAuthConfiguration): Intent {
|
||||
val serviceConfig = AuthorizationServiceConfiguration(
|
||||
config.authorizationEndpoint.toUri(),
|
||||
config.tokenEndpoint.toUri(),
|
||||
)
|
||||
|
||||
val authRequestBuilder = AuthorizationRequest.Builder(
|
||||
serviceConfig,
|
||||
config.clientId,
|
||||
ResponseTypeValues.CODE,
|
||||
config.redirectUri.toUri(),
|
||||
)
|
||||
|
||||
val scopeString = config.scopes.joinToString(separator = " ")
|
||||
val authRequest = authRequestBuilder
|
||||
.setScope(scopeString)
|
||||
.setLoginHint(email)
|
||||
.build()
|
||||
|
||||
val authService = getAuthService()
|
||||
|
||||
return authService.getAuthorizationRequestIntent(authRequest)
|
||||
}
|
||||
|
||||
private fun findOAuthConfiguration(account: Account): OAuthConfiguration? {
|
||||
return oAuthConfigurationProvider.getConfiguration(account.incomingServerSettings.host!!)
|
||||
is GetOAuthRequestIntentResult.Success -> resultObserver.login(authRequestIntentResult.intent)
|
||||
}
|
||||
}
|
||||
|
||||
private fun onLoginResult(authorizationResult: AuthorizationResult?) {
|
||||
|
|
|
@ -1,6 +1,5 @@
|
|||
package app.k9mail.core.common.oauth
|
||||
|
||||
interface OAuthConfigurationProvider {
|
||||
|
||||
fun interface OAuthConfigurationProvider {
|
||||
fun getConfiguration(hostname: String): OAuthConfiguration?
|
||||
}
|
||||
|
|
|
@ -0,0 +1,29 @@
|
|||
package app.k9mail.feature.account.oauth
|
||||
|
||||
import app.k9mail.core.common.coreCommonModule
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase
|
||||
import app.k9mail.feature.account.oauth.domain.usecase.GetOAuthRequestIntent
|
||||
import app.k9mail.feature.account.oauth.domain.usecase.SuggestServerName
|
||||
import net.openid.appauth.AuthorizationService
|
||||
import org.koin.android.ext.koin.androidApplication
|
||||
import org.koin.core.module.Module
|
||||
import org.koin.dsl.module
|
||||
|
||||
val featureAccountOAuthModule: Module = module {
|
||||
includes(coreCommonModule)
|
||||
|
||||
factory {
|
||||
AuthorizationService(
|
||||
androidApplication(),
|
||||
)
|
||||
}
|
||||
|
||||
factory<UseCase.SuggestServerName> { SuggestServerName() }
|
||||
|
||||
factory<UseCase.GetOAuthRequestIntent> {
|
||||
GetOAuthRequestIntent(
|
||||
service = get(),
|
||||
configurationProvider = get(),
|
||||
)
|
||||
}
|
||||
}
|
|
@ -1,10 +1,24 @@
|
|||
package app.k9mail.feature.account.oauth.domain
|
||||
|
||||
import android.content.Intent
|
||||
|
||||
interface DomainContract {
|
||||
|
||||
interface UseCase {
|
||||
fun interface SuggestServerName {
|
||||
fun suggest(protocol: String, domain: String): String
|
||||
}
|
||||
|
||||
fun interface GetOAuthRequestIntent {
|
||||
suspend fun execute(hostname: String, emailAddress: String): GetOAuthRequestIntentResult
|
||||
|
||||
sealed interface GetOAuthRequestIntentResult {
|
||||
object NotSupported : GetOAuthRequestIntentResult
|
||||
|
||||
data class Success(
|
||||
val intent: Intent,
|
||||
) : GetOAuthRequestIntentResult
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,46 @@
|
|||
package app.k9mail.feature.account.oauth.domain.usecase
|
||||
|
||||
import android.content.Intent
|
||||
import androidx.core.net.toUri
|
||||
import app.k9mail.core.common.oauth.OAuthConfiguration
|
||||
import app.k9mail.core.common.oauth.OAuthConfigurationProvider
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.GetOAuthRequestIntent
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.GetOAuthRequestIntent.GetOAuthRequestIntentResult
|
||||
import net.openid.appauth.AuthorizationRequest
|
||||
import net.openid.appauth.AuthorizationService
|
||||
import net.openid.appauth.AuthorizationServiceConfiguration
|
||||
import net.openid.appauth.ResponseTypeValues
|
||||
|
||||
internal class GetOAuthRequestIntent(
|
||||
private val service: AuthorizationService,
|
||||
private val configurationProvider: OAuthConfigurationProvider,
|
||||
) : GetOAuthRequestIntent {
|
||||
override suspend fun execute(hostname: String, emailAddress: String): GetOAuthRequestIntentResult {
|
||||
val configuration = configurationProvider.getConfiguration(hostname)
|
||||
?: return GetOAuthRequestIntentResult.NotSupported
|
||||
|
||||
return GetOAuthRequestIntentResult.Success(createAuthorizationRequestIntent(emailAddress, configuration))
|
||||
}
|
||||
|
||||
private fun createAuthorizationRequestIntent(emailAddress: String, configuration: OAuthConfiguration): Intent {
|
||||
val serviceConfig = AuthorizationServiceConfiguration(
|
||||
configuration.authorizationEndpoint.toUri(),
|
||||
configuration.tokenEndpoint.toUri(),
|
||||
)
|
||||
|
||||
val authRequestBuilder = AuthorizationRequest.Builder(
|
||||
serviceConfig,
|
||||
configuration.clientId,
|
||||
ResponseTypeValues.CODE,
|
||||
configuration.redirectUri.toUri(),
|
||||
)
|
||||
|
||||
val authRequest = authRequestBuilder
|
||||
.setScope(configuration.scopes.joinToString(" "))
|
||||
.setCodeVerifier(null)
|
||||
.setLoginHint(emailAddress)
|
||||
.build()
|
||||
|
||||
return service.getAuthorizationRequestIntent(authRequest)
|
||||
}
|
||||
}
|
|
@ -3,6 +3,7 @@ package app.k9mail.feature.account.oauth.domain.usecase
|
|||
import app.k9mail.core.common.mail.Protocols
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase
|
||||
|
||||
@Deprecated("This is not needed anymore, remove once auth setup flow is updated")
|
||||
class SuggestServerName : UseCase.SuggestServerName {
|
||||
override fun suggest(protocol: String, domain: String): String = when (protocol) {
|
||||
Protocols.IMAP -> "imap.$domain"
|
||||
|
|
|
@ -0,0 +1,92 @@
|
|||
package app.k9mail.feature.account.oauth.domain.usecase
|
||||
|
||||
import android.content.Intent
|
||||
import androidx.core.net.toUri
|
||||
import app.k9mail.core.common.oauth.OAuthConfiguration
|
||||
import app.k9mail.feature.account.oauth.domain.DomainContract.UseCase.GetOAuthRequestIntent
|
||||
import assertk.all
|
||||
import assertk.assertThat
|
||||
import assertk.assertions.isEqualTo
|
||||
import assertk.assertions.isNull
|
||||
import assertk.assertions.prop
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import net.openid.appauth.AuthorizationRequest
|
||||
import net.openid.appauth.AuthorizationService
|
||||
import net.openid.appauth.AuthorizationServiceConfiguration
|
||||
import net.openid.appauth.ResponseTypeValues
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.mockito.kotlin.argumentCaptor
|
||||
import org.mockito.kotlin.mock
|
||||
import org.mockito.kotlin.stub
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class GetOAuthRequestIntentTest {
|
||||
|
||||
private val service: AuthorizationService = mock<AuthorizationService>()
|
||||
|
||||
@Test
|
||||
fun `should return NotSupported when hostname has no oauth configuration`() = runTest {
|
||||
val testSubject = GetOAuthRequestIntent(
|
||||
service = service,
|
||||
configurationProvider = { null },
|
||||
)
|
||||
val hostname = "hostname"
|
||||
val emailAddress = "emailAddress"
|
||||
|
||||
val result = testSubject.execute(hostname, emailAddress)
|
||||
|
||||
assertThat(result).isEqualTo(GetOAuthRequestIntent.GetOAuthRequestIntentResult.NotSupported)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `should return Success with intent when hostname has oauth configuration`() = runTest {
|
||||
val testSubject = GetOAuthRequestIntent(
|
||||
service = service,
|
||||
configurationProvider = { oAuthConfiguration },
|
||||
)
|
||||
val hostname = "hostname"
|
||||
val emailAddress = "emailAddress"
|
||||
val intent = Intent()
|
||||
val authRequestCapture = argumentCaptor<AuthorizationRequest>().apply {
|
||||
service.stub { on { getAuthorizationRequestIntent(capture()) }.thenReturn(intent) }
|
||||
}
|
||||
|
||||
// When
|
||||
val result = testSubject.execute(hostname, emailAddress)
|
||||
|
||||
// Then
|
||||
assertThat(result).isEqualTo(
|
||||
GetOAuthRequestIntent.GetOAuthRequestIntentResult.Success(
|
||||
intent = intent,
|
||||
),
|
||||
)
|
||||
assertThat(authRequestCapture.firstValue).all {
|
||||
prop(AuthorizationRequest::configuration).all {
|
||||
prop(AuthorizationServiceConfiguration::authorizationEndpoint).isEqualTo(
|
||||
oAuthConfiguration.authorizationEndpoint.toUri(),
|
||||
)
|
||||
prop(AuthorizationServiceConfiguration::tokenEndpoint).isEqualTo(
|
||||
oAuthConfiguration.tokenEndpoint.toUri(),
|
||||
)
|
||||
}
|
||||
prop(AuthorizationRequest::clientId).isEqualTo(oAuthConfiguration.clientId)
|
||||
prop(AuthorizationRequest::responseType).isEqualTo(ResponseTypeValues.CODE)
|
||||
prop(AuthorizationRequest::redirectUri).isEqualTo(oAuthConfiguration.redirectUri.toUri())
|
||||
prop(AuthorizationRequest::scope).isEqualTo("scope scope2")
|
||||
prop(AuthorizationRequest::codeVerifier).isNull()
|
||||
prop(AuthorizationRequest::loginHint).isEqualTo(emailAddress)
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
val oAuthConfiguration = OAuthConfiguration(
|
||||
clientId = "clientId",
|
||||
scopes = listOf("scope", "scope2"),
|
||||
authorizationEndpoint = "auth.example.com",
|
||||
tokenEndpoint = "token.example.com",
|
||||
redirectUri = "redirect.example.com",
|
||||
)
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue