diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 4822560b94a..ad36ca26107 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -166,8 +166,6 @@ kotlin { // reorderable lists (raw Compose API is pretty complicated) implementation("sh.calvin.reorderable:reorderable:2.5.1") - // multiplatform webview (for login via OAuth) - implementation("io.github.kevinnzou:compose-webview-multiplatform:2.0.3") // sharing presets/settings via QR Code implementation("io.github.alexzhirkevich:qrose:1.0.1") @@ -214,7 +212,7 @@ kotlin { implementation("com.github.chrisbanes:PhotoView:2.3.0") // map and location - implementation("org.maplibre.gl:android-sdk:12.3.1") + implementation("org.maplibre.gl:android-sdk:12.1.0") } } iosMain { diff --git a/app/src/androidMain/AndroidManifest.xml b/app/src/androidMain/AndroidManifest.xml index b347cb32719..51e8177d3cb 100644 --- a/app/src/androidMain/AndroidManifest.xml +++ b/app/src/androidMain/AndroidManifest.xml @@ -34,6 +34,7 @@ + by inject(named("FeatureDictionaryLazy")) private val soundFx: SoundFx by inject() + private val oAuthCallbackHandler: OAuthCallbackHandler by inject() + private val oAuthLoginCompleter: OAuthLoginCompleter by inject() + private val userLoginSource: UserLoginSource by inject() private lateinit var locationManager: FineLocationManager @@ -302,8 +308,31 @@ class MainActivity : private fun handleIntent(intent: Intent) { if (intent.action != Intent.ACTION_VIEW) return - val data = intent.data?.toString() ?: return - viewModel.setUri(data) + val data = intent.data ?: return + val dataString = data.toString() + if (oAuthCallbackHandler.handleUri(dataString)) { + lifecycleScope.launch { + val success = oAuthLoginCompleter.processCallback(dataString) + withContext(Dispatchers.Main) { + if (success) { + val userIntent = Intent(this@MainActivity, de.westnordost.streetcomplete.screens.user.UserActivity::class.java) + startActivity(userIntent) + } else { + // In some flows the user may already be logged in (e.g. external browser finished auth) + val isLoggedIn = userLoginSource.isLoggedIn + if (isLoggedIn) { + val userIntent = Intent(this@MainActivity, de.westnordost.streetcomplete.screens.user.UserActivity::class.java) + startActivity(userIntent) + } else { + toast(R.string.oauth_communication_error, Toast.LENGTH_LONG) + } + } + } + } + return + } + + viewModel.setUri(dataString) } override fun onConfigurationChanged(newConfig: Configuration) { diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginScreen.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginScreen.kt index 2b503ddc5e2..d6201b4371b 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginScreen.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginScreen.kt @@ -4,14 +4,9 @@ import android.widget.Toast import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.WindowInsets -import androidx.compose.foundation.layout.WindowInsetsSides import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.only import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.safeDrawing -import androidx.compose.foundation.layout.windowInsetsPadding import androidx.compose.material.AppBarDefaults import androidx.compose.material.Button import androidx.compose.material.ContentAlpha @@ -28,30 +23,20 @@ import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.text.intl.Locale +import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.text.style.TextAlign -import androidx.compose.ui.unit.dp -import com.multiplatform.webview.request.RequestInterceptor -import com.multiplatform.webview.request.WebRequest -import com.multiplatform.webview.request.WebRequestInterceptResult -import com.multiplatform.webview.web.LoadingState -import com.multiplatform.webview.web.WebView -import com.multiplatform.webview.web.WebViewNavigator -import com.multiplatform.webview.web.rememberWebViewNavigator -import com.multiplatform.webview.web.rememberWebViewState -import de.westnordost.streetcomplete.ApplicationConstants -import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.resources.Res import de.westnordost.streetcomplete.resources.unsynced_quests_not_logged_in_description import de.westnordost.streetcomplete.resources.user_login +import androidx.compose.ui.unit.dp +import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.screens.user.login.LoginError.CommunicationError import de.westnordost.streetcomplete.screens.user.login.LoginError.RequiredPermissionsNotGranted import de.westnordost.streetcomplete.ui.common.BackIcon import de.westnordost.streetcomplete.ui.theme.titleLarge import de.westnordost.streetcomplete.util.ktx.toast import org.jetbrains.compose.resources.stringResource - -/** Leads user through the OAuth 2 auth flow to login */ +/** Leads user through the OAuth 2 auth flow to login using the external browser */ @Composable fun LoginScreen( viewModel: LoginViewModel, @@ -62,7 +47,9 @@ fun LoginScreen( val unsyncedChangesCount by viewModel.unsyncedChangesCount.collectAsState() LaunchedEffect(launchAuth) { - if (launchAuth) viewModel.startLogin() + if (launchAuth) { + viewModel.startLogin() + } } // handle error state: just show message once and return to login state @@ -79,11 +66,37 @@ fun LoginScreen( } } + // Launch external browser for OAuth when requesting authorization + val uriHandler = LocalUriHandler.current + LaunchedEffect(state) { + if (state is RequestingAuthorization && !viewModel.hasAuthUrlLaunched()) { + viewModel.markAuthUrlLaunched() + val authUrl = viewModel.authorizationRequestUrl + uriHandler.openUri(authUrl) + } + } + + LaunchedEffect(state) { + if(state is RequestingAuthorization && viewModel.loginState.value is RequestingAuthorization){ + viewModel.resetLogin() + } + } + Column(Modifier.fillMaxSize()) { TopAppBar( title = { Text(stringResource(Res.string.user_login)) }, windowInsets = AppBarDefaults.topAppBarWindowInsets, - navigationIcon = { IconButton(onClick = onClickBack) { BackIcon() } }, + navigationIcon = { + IconButton(onClick = { + // If user navigates back while waiting for browser auth, reset loading state + if (state is RequestingAuthorization) { + viewModel.resetLogin() + } + onClickBack() + }) { + BackIcon() + } + }, ) if (state is LoggedOut) { @@ -93,62 +106,9 @@ fun LoginScreen( modifier = Modifier.fillMaxSize() ) } else if (state is RequestingAuthorization) { - val webViewState = rememberWebViewState( - url = viewModel.authorizationRequestUrl, - additionalHttpHeaders = mapOf( - "Accept-Language" to Locale.current.toLanguageTag() - ) - ) - - val webViewNavigator = rememberWebViewNavigator( - // handle authorization url response - requestInterceptor = object : RequestInterceptor { - override fun onInterceptUrlRequest( - request: WebRequest, - navigator: WebViewNavigator - ): WebRequestInterceptResult { - if (viewModel.isAuthorizationResponseUrl(request.url)) { - viewModel.finishAuthorization(request.url) - return WebRequestInterceptResult.Reject - } - return WebRequestInterceptResult.Allow - } - } - ) - - // handle error response - LaunchedEffect(webViewState.errorsForCurrentRequest) { - val error = webViewState.errorsForCurrentRequest.firstOrNull() - if (error != null) { - viewModel.failAuthorization( - url = webViewState.lastLoadedUrl.toString(), - errorCode = error.code, - description = error.description - ) - } - } - - Box(Modifier - .fillMaxSize() - .windowInsetsPadding(WindowInsets.safeDrawing.only( - WindowInsetsSides.Horizontal + WindowInsetsSides.Bottom - )) - ) { - if (webViewState.loadingState is LoadingState.Loading) { - LinearProgressIndicator(Modifier.fillMaxWidth()) - } - WebView( - state = webViewState, - modifier = Modifier.fillMaxSize(), - captureBackPresses = true, - navigator = webViewNavigator, - onCreated = { - val settings = webViewState.webSettings - settings.isJavaScriptEnabled = true - settings.customUserAgentString = ApplicationConstants.USER_AGENT - settings.supportZoom = false - } as () -> Unit, - ) + // Show the loading state while browser is handling authorization + Box(Modifier.fillMaxSize()) { + LinearProgressIndicator(Modifier.fillMaxWidth()) } } else if (state is RetrievingAccessToken || state is LoggedIn) { Box(Modifier.fillMaxSize()) { diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthCallbackHandler.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthCallbackHandler.kt new file mode 100644 index 00000000000..21cc5639aca --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthCallbackHandler.kt @@ -0,0 +1,54 @@ +package de.westnordost.streetcomplete.data.user + +import de.westnordost.streetcomplete.data.user.oauth.OAuthAuthorizationParams +import de.westnordost.streetcomplete.util.logs.Log +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.getAndUpdate +import kotlinx.coroutines.flow.update + +/** + * When the user completes the OAuth flow and returns to the app via the callback URI, + * this handler processes the authorization response. + */ +class OAuthCallbackHandler { + private val _oAuthCallbackUri = MutableStateFlow(null) + val oAuthCallbackUri: StateFlow = _oAuthCallbackUri + + // Store the OAuth params so the same codeVerifier can be used during token exchange + private var storedOAuthParams: OAuthAuthorizationParams? = null + + fun storeOAuthParams(params: OAuthAuthorizationParams) { + storedOAuthParams = params + } + + fun getStoredOAuthParams(): OAuthAuthorizationParams? = storedOAuthParams + + // Process a potential OAuth callback URI + fun handleUri(uriString: String): Boolean { + return if (isOAuthCallback(uriString)) { + _oAuthCallbackUri.update { uriString } + true + } else { + false + } + } + + // checks if the uri is oauth callback + private fun isOAuthCallback(uriString: String): Boolean { + return uriString.startsWith("$OAUTH2_CALLBACK_SCHEME://$OAUTH2_CALLBACK_HOST") + } + + fun consumeCallback(): String? { + return _oAuthCallbackUri.getAndUpdate { null } + } + + fun clearStoredParams() { + storedOAuthParams = null + } + + companion object { + private const val TAG = "OAuthCallbackHandler" + } +} + diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthLoginCompleter.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthLoginCompleter.kt new file mode 100644 index 00000000000..594dc49740c --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/OAuthLoginCompleter.kt @@ -0,0 +1,44 @@ +package de.westnordost.streetcomplete.data.user + +import de.westnordost.streetcomplete.data.user.oauth.OAuthApiClient +import de.westnordost.streetcomplete.data.user.oauth.OAuthException +import de.westnordost.streetcomplete.util.logs.Log + + +// Finishes the OAuth login flow when an authorization callback URL is received. +class OAuthLoginCompleter( + private val oAuthApiClient: OAuthApiClient, + private val userLoginController: UserLoginController, + private val oAuthCallbackHandler: OAuthCallbackHandler +) { + + suspend fun processCallback(authorizationResponseUrl: String): Boolean { + + val oAuthParams = oAuthCallbackHandler.getStoredOAuthParams() ?: return false + + return try { + val tokenResponse = oAuthApiClient.getAccessToken(oAuthParams, authorizationResponseUrl) + if (tokenResponse.grantedScopes?.containsAll(OAUTH2_REQUIRED_SCOPES) == false) { + return false + } + + userLoginController.logIn(tokenResponse.accessToken) + + // Clear stored params after successful login + oAuthCallbackHandler.clearStoredParams() + true + } catch (e: Exception) { + if (e is OAuthException && e.error == "access_denied") { + Log.w(TAG, "OAuth access denied by user") + } else { + Log.e(TAG, "Error finishing OAuth login: ${e.message}", e) + } + false + } + } + + companion object { + private const val TAG = "OAuthLoginCompleter" + } +} + diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserLoginController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserLoginController.kt index 20ec2c62e88..387c3df8fa0 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserLoginController.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserLoginController.kt @@ -5,6 +5,7 @@ import de.westnordost.streetcomplete.util.Listeners class UserLoginController( private val prefs: Preferences, + private val oAuthCallbackHandler: OAuthCallbackHandler, ) : UserLoginSource { private val listeners = Listeners() @@ -22,6 +23,8 @@ class UserLoginController( fun logOut() { prefs.oAuth2AccessToken = null prefs.removeOAuth1Data() + oAuthCallbackHandler.consumeCallback() + oAuthCallbackHandler.clearStoredParams() listeners.forEach { it.onLoggedOut() } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserModule.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserModule.kt index 9d21a973bd8..474e27af580 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserModule.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/UserModule.kt @@ -47,11 +47,14 @@ val userModule = module { single { get() } single { UserDataController(get(), get()) } + single { OAuthApiClient(get()) } + single { OAuthCallbackHandler() } + single { get() } single { get() } - single { UserLoginController(get()) } + single { UserLoginController(get(), get()) } single { UserUpdater(get(), get(), get(), get(), get(), get()) } - single { OAuthApiClient(get()) } + single { OAuthLoginCompleter(get(), get(), get()) } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/oauth/OAuthApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/oauth/OAuthApiClient.kt index a980e251782..294141a270f 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/oauth/OAuthApiClient.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/user/oauth/OAuthApiClient.kt @@ -107,8 +107,9 @@ class OAuthApiClient(private val httpClient: HttpClient) { * Creates the URL to be opened in the browser or a web view in which the user agrees to * authorize the requested permissions. */ - val authorizationRequestUrl get() = - URLBuilder().takeFrom(authorizationUrl).apply { + val authorizationRequestUrl: String get() { + // Build the OAuth authorize URL with all parameters + val oAuthUrl = URLBuilder().takeFrom(authorizationUrl).apply { parameters.append("response_type", "code") parameters.append("client_id", clientId) parameters.append("scope", scopes.joinToString(" ")) @@ -118,6 +119,9 @@ class OAuthApiClient(private val httpClient: HttpClient) { state?.let { parameters.append("state", it) } }.build().toString() + return oAuthUrl + } + /** * Checks whether the given callback uri is meant for this instance */ diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/UserScreenModule.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/UserScreenModule.kt index ffd7619990f..ab78a500fe4 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/UserScreenModule.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/UserScreenModule.kt @@ -19,7 +19,7 @@ val userScreenModule = module { get(), get(), get(), get(), get(), get(), get(), get(named("AvatarsCacheDirectory")) ) } - viewModel { LoginViewModelImpl(get(), get(), get()) } + viewModel { LoginViewModelImpl(get(), get(), get(), get()) } viewModel { EditStatisticsViewModelImpl(get(), get(), get()) } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginViewModel.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginViewModel.kt index ac64b22ddf0..5712c5d204b 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginViewModel.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/screens/user/login/LoginViewModel.kt @@ -9,6 +9,7 @@ import de.westnordost.streetcomplete.data.user.OAUTH2_REDIRECT_URI import de.westnordost.streetcomplete.data.user.OAUTH2_REQUESTED_SCOPES import de.westnordost.streetcomplete.data.user.OAUTH2_REQUIRED_SCOPES import de.westnordost.streetcomplete.data.user.OAUTH2_TOKEN_URL +import de.westnordost.streetcomplete.data.user.OAuthCallbackHandler import de.westnordost.streetcomplete.data.user.UserLoginController import de.westnordost.streetcomplete.data.user.UserLoginSource import de.westnordost.streetcomplete.data.user.oauth.OAuthApiClient @@ -33,17 +34,15 @@ abstract class LoginViewModel : ViewModel() { /** Starts the OAuth2 based login flow. */ abstract fun startLogin() - /** Call when the web view / browser received an error when loading the (authorization) page */ - abstract fun failAuthorization(url: String, errorCode: Int, description: String?) - - /** Returns whether the url is a redirect url destined for this OAuth authorization flow */ - abstract fun isAuthorizationResponseUrl(url: String): Boolean - - /** Continues OAuth authorization flow with given redirect url */ - abstract fun finishAuthorization(authorizationResponseUrl: String) /** Resets the login state to LoggedOut. Only works if current state is LoginError */ abstract fun resetLogin() + + /** Marks that the external auth URL has been launched to prevent re-launching on recomposition */ + abstract fun markAuthUrlLaunched() + + /** Check if auth URL was already launched */ + abstract fun hasAuthUrlLaunched(): Boolean } sealed interface LoginState @@ -60,7 +59,8 @@ data object LoggedIn : LoginState class LoginViewModelImpl( private val unsyncedChangesCountSource: UnsyncedChangesCountSource, private val userLoginController: UserLoginController, - private val oAuthApiClient: OAuthApiClient + private val oAuthApiClient: OAuthApiClient, + private val oAuthCallbackHandler: OAuthCallbackHandler ) : LoginViewModel() { override val loginState = MutableStateFlow(LoggedOut) override val unsyncedChangesCount = MutableStateFlow(0) @@ -75,9 +75,17 @@ class LoginViewModelImpl( OAUTH2_REDIRECT_URI ) + private var authUrlLaunched = false + private val loginStatusListener = object : UserLoginSource.Listener { - override fun onLoggedIn() { loginState.value = LoggedIn } - override fun onLoggedOut() { loginState.value = LoggedOut } + override fun onLoggedIn() { + loginState.value = LoggedIn + authUrlLaunched = false + } + override fun onLoggedOut() { + loginState.value = LoggedOut + authUrlLaunched = false + } } init { @@ -85,6 +93,20 @@ class LoginViewModelImpl( unsyncedChangesCount.update { unsyncedChangesCountSource.getCount() } } userLoginController.addListener(loginStatusListener) + + // Monitor for the oauth callbacks + launch { + oAuthCallbackHandler.oAuthCallbackUri.collect { callbackUri -> + if (callbackUri != null) { + // if state was reset (e.g. activity recreated) we should be in the requesting state + if (loginState.value !is RequestingAuthorization && loginState.value !is RetrievingAccessToken && loginState.value !is LoggedIn) { + loginState.value = RequestingAuthorization + } + finishAuthorization(callbackUri) + oAuthCallbackHandler.consumeCallback() + } + } + } } override fun onCleared() { @@ -92,53 +114,45 @@ class LoginViewModelImpl( } override fun startLogin() { + oAuthCallbackHandler.storeOAuthParams(oAuth) loginState.compareAndSet(LoggedOut, RequestingAuthorization) + authUrlLaunched = false } - override fun failAuthorization(url: String, errorCode: Int, description: String?) { - Log.e(TAG, "Error for URL " + url + if (description != null) ": $description" else "") - loginState.compareAndSet(RequestingAuthorization, LoginError.CommunicationError) - } - - override fun isAuthorizationResponseUrl(url: String): Boolean = - oAuth.itsForMe(url) - - override fun finishAuthorization(authorizationResponseUrl: String) { + private fun finishAuthorization(authorizationResponseUrl: String) { launch { - val accessToken = retrieveAccessToken(authorizationResponseUrl) - if (accessToken != null) { - login(accessToken) + try { + loginState.value = RetrievingAccessToken + val accessTokenResponse = oAuthApiClient.getAccessToken(oAuth, authorizationResponseUrl) + if (accessTokenResponse.grantedScopes?.containsAll(OAUTH2_REQUIRED_SCOPES) == false) { + loginState.value = LoginError.RequiredPermissionsNotGranted + return@launch + } + userLoginController.logIn(accessTokenResponse.accessToken) + } catch (e: Exception) { + if (e is OAuthException && e.error == "access_denied") { + loginState.value = LoginError.RequiredPermissionsNotGranted + } else { + Log.e(TAG, "Error during authorization", e) + loginState.value = if (userLoginController.isLoggedIn) LoggedIn else LoginError.CommunicationError + } } } } - private suspend fun retrieveAccessToken(authorizationResponseUrl: String): String? { - try { - loginState.value = RetrievingAccessToken - val accessTokenResponse = oAuthApiClient.getAccessToken(oAuth, authorizationResponseUrl) - if (accessTokenResponse.grantedScopes?.containsAll(OAUTH2_REQUIRED_SCOPES) == false) { - loginState.value = LoginError.RequiredPermissionsNotGranted - return null - } - return accessTokenResponse.accessToken - } catch (e: Exception) { - if (e is OAuthException && e.error == "access_denied") { - loginState.value = LoginError.RequiredPermissionsNotGranted - } else { - Log.e(TAG, "Error during authorization", e) - loginState.value = LoginError.CommunicationError - } - return null + override fun resetLogin() { + // This handles the case where user closes the browser without completing auth + if (loginState.value is LoginError || loginState.value is RequestingAuthorization) { + loginState.value = LoggedOut + authUrlLaunched = false } } - private suspend fun login(accessToken: String) { - userLoginController.logIn(accessToken) + override fun markAuthUrlLaunched() { + authUrlLaunched = true } - override fun resetLogin() { - if (loginState.value is LoginError) loginState.value = LoggedOut - } + override fun hasAuthUrlLaunched(): Boolean = authUrlLaunched companion object { private const val TAG = "Login"