From 1429327c69db35cfe289bfa52e571d3269c078d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolf-Martell=20Montwe=CC=81?= Date: Mon, 26 Jun 2023 12:39:07 +0200 Subject: [PATCH] Extract GetOAuthRequestIntent into own use case for use in the new account flow --- .../main/java/com/fsck/k9/UiKoinModules.kt | 2 + .../java/com/fsck/k9/activity/KoinModule.kt | 10 +- .../activity/setup/AccountSetupAccountType.kt | 2 +- .../fsck/k9/activity/setup/AuthViewModel.kt | 58 +++--------- .../oauth/OAuthConfigurationProvider.kt | 3 +- .../account/oauth/AccountOAuthModule.kt | 29 ++++++ .../account/oauth/domain/DomainContract.kt | 14 +++ .../domain/usecase/GetOAuthRequestIntent.kt | 46 ++++++++++ .../oauth/domain/usecase/SuggestServerName.kt | 1 + .../usecase/GetOAuthRequestIntentTest.kt | 92 +++++++++++++++++++ 10 files changed, 206 insertions(+), 51 deletions(-) create mode 100644 feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/AccountOAuthModule.kt create mode 100644 feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntent.kt create mode 100644 feature/account/oauth/src/test/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntentTest.kt diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/UiKoinModules.kt b/app/ui/legacy/src/main/java/com/fsck/k9/UiKoinModules.kt index d1b3ae31374..f4fa4c71db3 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/UiKoinModules.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/UiKoinModules.kt @@ -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, diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/activity/KoinModule.kt b/app/ui/legacy/src/main/java/com/fsck/k9/activity/KoinModule.kt index da61a9ac111..3663bc8dbf8 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/activity/KoinModule.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/activity/KoinModule.kt @@ -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(), + ) + } } diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AccountSetupAccountType.kt b/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AccountSetupAccountType.kt index 7761ed84fa9..29d1b105fb4 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AccountSetupAccountType.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AccountSetupAccountType.kt @@ -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 diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AuthViewModel.kt b/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AuthViewModel.kt index aa29e6daed4..5d165873cb6 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AuthViewModel.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/activity/setup/AuthViewModel.kt @@ -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) - } - - 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) - } + when (authRequestIntentResult) { + GetOAuthRequestIntentResult.NotSupported -> { + _uiState.update { AuthFlowState.NotSupported } + } - private fun findOAuthConfiguration(account: Account): OAuthConfiguration? { - return oAuthConfigurationProvider.getConfiguration(account.incomingServerSettings.host!!) + is GetOAuthRequestIntentResult.Success -> resultObserver.login(authRequestIntentResult.intent) + } } private fun onLoginResult(authorizationResult: AuthorizationResult?) { diff --git a/core/common/src/main/kotlin/app/k9mail/core/common/oauth/OAuthConfigurationProvider.kt b/core/common/src/main/kotlin/app/k9mail/core/common/oauth/OAuthConfigurationProvider.kt index cc6c4960241..ad29f86cdae 100644 --- a/core/common/src/main/kotlin/app/k9mail/core/common/oauth/OAuthConfigurationProvider.kt +++ b/core/common/src/main/kotlin/app/k9mail/core/common/oauth/OAuthConfigurationProvider.kt @@ -1,6 +1,5 @@ package app.k9mail.core.common.oauth -interface OAuthConfigurationProvider { - +fun interface OAuthConfigurationProvider { fun getConfiguration(hostname: String): OAuthConfiguration? } diff --git a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/AccountOAuthModule.kt b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/AccountOAuthModule.kt new file mode 100644 index 00000000000..ed380d15c8e --- /dev/null +++ b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/AccountOAuthModule.kt @@ -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 { SuggestServerName() } + + factory { + GetOAuthRequestIntent( + service = get(), + configurationProvider = get(), + ) + } +} diff --git a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/DomainContract.kt b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/DomainContract.kt index 8f98df0be4e..8e08e22f301 100644 --- a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/DomainContract.kt +++ b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/DomainContract.kt @@ -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 + } + } } } diff --git a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntent.kt b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntent.kt new file mode 100644 index 00000000000..4eec931a52f --- /dev/null +++ b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntent.kt @@ -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) + } +} diff --git a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/SuggestServerName.kt b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/SuggestServerName.kt index 731a0937e1d..6125730642a 100644 --- a/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/SuggestServerName.kt +++ b/feature/account/oauth/src/main/kotlin/app/k9mail/feature/account/oauth/domain/usecase/SuggestServerName.kt @@ -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" diff --git a/feature/account/oauth/src/test/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntentTest.kt b/feature/account/oauth/src/test/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntentTest.kt new file mode 100644 index 00000000000..99d8267af9c --- /dev/null +++ b/feature/account/oauth/src/test/kotlin/app/k9mail/feature/account/oauth/domain/usecase/GetOAuthRequestIntentTest.kt @@ -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() + + @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().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", + ) + } +}