From bf76a469f163069f9c47aa572bc2617d804486ae Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 22 Apr 2025 21:18:31 +0600 Subject: [PATCH 01/13] feat: refresh authState before notes api calls If user logged-in using oidc for murena account, the accessToken is only refreshed on caldav & carddav sync happens (forcefully by user, or periodic sync in every ~15 mins). Notes app reuse the same token, don't check/update the token. In a corner case, when dav syncs is not happening in next X minutes, but the token TTL is ended, & user opens the notes app, then the apis will return 401. To resolve this, we need to refresh the token before the notes api request is made. AppAuth lib use callBack function to notify the token refresh is done. By we want it to be synchornous because notes app request is actually synchronous. We have access this by using kotlin's runBlocking & future features. --- .../android/sso/InputStreamBinder.java | 4 + .../android/sso/OIDCTokenRefresher.kt | 93 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 5f45166d2..975158afa 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -335,6 +335,8 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } + OIDCTokenRefresher.Companion.refresh(context, account); + final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); final OwnCloudClient client = ownCloudClientManager.getClientFor(ownCloudAccount, context, OwnCloudClient.DONT_USE_COOKIES); @@ -424,6 +426,8 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } + OIDCTokenRefresher.Companion.refresh(context, account); + final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); final OwnCloudClient client = ownCloudClientManager.getClientFor(ownCloudAccount, context, OwnCloudClient.DONT_USE_COOKIES); diff --git a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt new file mode 100644 index 000000000..ea69c8d2a --- /dev/null +++ b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt @@ -0,0 +1,93 @@ +/* + * Copyright MURENA SAS 2023 + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.nextcloud.android.sso + +import android.accounts.Account +import android.content.Context +import at.bitfire.davdroid.OpenIdUtils +import at.bitfire.davdroid.log.Logger +import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint +import at.bitfire.davdroid.settings.AccountSettings +import com.nextcloud.android.utils.AccountManagerUtils +import dagger.hilt.android.EntryPointAccessors +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.runBlocking +import net.openid.appauth.AuthState +import net.openid.appauth.AuthorizationException +import net.openid.appauth.AuthorizationService +import net.openid.appauth.ClientAuthentication +import java.util.logging.Level + +class OIDCTokenRefresher { + companion object { + + fun refresh(context: Context, account: Account) { + val isOidcAccount = AccountManagerUtils.isOidcAccount(context, account) + if (!isOidcAccount) { + return + } + + val accountSettings = AccountSettings(context, account) + val credentials = accountSettings.credentials() + if (credentials.authState == null) { + return + } + + val authorizationService = EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java).authorizationService() + val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret) + val updatedAuthState = refreshAuthState(authorizationService, credentials.authState, clientAuth) + + if (updatedAuthState != null) { + accountSettings.credentials( + accountSettings.credentials().copy( + authState = updatedAuthState + ) + ) + } + } + + private fun refreshAuthState( + authService: AuthorizationService, + authState: AuthState, + clientAuth: ClientAuthentication + ): AuthState? { + return runBlocking { + val authStateFuture = CompletableDeferred() + + authState.performActionWithFreshTokens( + authService, + clientAuth + ) { accessToken: String?, _: String?, ex: AuthorizationException? -> + if (accessToken != null) { + authStateFuture.complete(authState) + } else { + Logger.log.log(Level.WARNING, "Couldn't refresh access token", ex) + authStateFuture.cancel() + } + } + + // return value + try { + authStateFuture.await() + } catch (ignored: CancellationException) { + null + } + } + } + } +} -- GitLab From 21da89fa92ad22c088cd2c913d5bb2e90aad7c10 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 22 Apr 2025 21:19:47 +0600 Subject: [PATCH 02/13] chore: update nc-android-lib version to 2.0.1-u2.17 - fix notes api doesn't using latest oidc accessTokens --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index 86d38ef7d..ca65f42d9 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -146,7 +146,7 @@ android { buildConfigField "String", "GOOGLE_CLIENT_ID", "\"${retrieveKey("GOOGLE_CLIENT_ID")}\"" buildConfigField "String", "GOOGLE_REDIRECT_URI", "\"${retrieveKey("GOOGLE_REDIRECT_URI")}\"" - + buildConfigField "String", "MURENA_CLIENT_SECRET", "\"${retrieveKey("MURENA_CLIENT_SECRET")}\"" buildConfigField "String", "YAHOO_CLIENT_ID", "\"${retrieveKey("YAHOO_CLIENT_ID")}\"" manifestPlaceholders = [ -- GitLab From 4b764285673323d963c792af267ad4912a25682d Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 22 Apr 2025 21:25:52 +0600 Subject: [PATCH 03/13] fix: remove unwanted murena_client_secret value - remove client_secret from murena sso setup as it not needed & wrongfully added by previous commit - refactor code according to review (https://gitlab.e.foundation/e/os/AccountManager/-/merge_requests/155) --- app/build.gradle | 2 +- .../android/sso/OIDCTokenRefresher.kt | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index ca65f42d9..86d38ef7d 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -146,7 +146,7 @@ android { buildConfigField "String", "GOOGLE_CLIENT_ID", "\"${retrieveKey("GOOGLE_CLIENT_ID")}\"" buildConfigField "String", "GOOGLE_REDIRECT_URI", "\"${retrieveKey("GOOGLE_REDIRECT_URI")}\"" - buildConfigField "String", "MURENA_CLIENT_SECRET", "\"${retrieveKey("MURENA_CLIENT_SECRET")}\"" + buildConfigField "String", "YAHOO_CLIENT_ID", "\"${retrieveKey("YAHOO_CLIENT_ID")}\"" manifestPlaceholders = [ diff --git a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt index ea69c8d2a..7a5f7d84e 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt @@ -1,5 +1,5 @@ /* - * Copyright MURENA SAS 2023 + * Copyright MURENA SAS 2025 * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation, either version 3 of the License, or @@ -37,8 +37,7 @@ class OIDCTokenRefresher { companion object { fun refresh(context: Context, account: Account) { - val isOidcAccount = AccountManagerUtils.isOidcAccount(context, account) - if (!isOidcAccount) { + if (!AccountManagerUtils.isOidcAccount(context, account)) { return } @@ -50,15 +49,17 @@ class OIDCTokenRefresher { val authorizationService = EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java).authorizationService() val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret) + val updatedAuthState = refreshAuthState(authorizationService, credentials.authState, clientAuth) + if (updatedAuthState == null) { + return + } - if (updatedAuthState != null) { - accountSettings.credentials( - accountSettings.credentials().copy( - authState = updatedAuthState - ) + accountSettings.credentials( + accountSettings.credentials().copy( + authState = updatedAuthState ) - } + ) } private fun refreshAuthState( -- GitLab From 38061357672782469d07209b90bc9b61948c1d4b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 23 Apr 2025 12:08:07 +0600 Subject: [PATCH 04/13] chore: update according to code review MR: https://gitlab.e.foundation/e/os/AccountManager/-/merge_requests/155 --- .../android/sso/InputStreamBinder.java | 13 ++++++------- .../nextcloud/android/sso/OIDCTokenRefresher.kt | 17 +++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 975158afa..9e7fc078a 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -80,7 +80,6 @@ import java.util.List; import java.util.Map; import java.util.logging.Level; -import at.bitfire.davdroid.BuildConfig; import at.bitfire.davdroid.log.Logger; public class InputStreamBinder extends IInputStreamService.Stub { @@ -335,7 +334,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - OIDCTokenRefresher.Companion.refresh(context, account); + OIDCTokenRefresher.refresh(context, account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); @@ -392,10 +391,10 @@ public class InputStreamBinder extends IInputStreamService.Stub { } /* - * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. - * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. - * - * These nextcloud app request paths contain `/index.php/apps/` on them. + * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. + * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. + * + * These nextcloud app request paths contain `/index.php/apps/` on them. */ private boolean shouldAddHeaderForOidcLogin(@NonNull Context context, @NonNull Account account, @NonNull String path) { boolean isOidcAccount = AccountManagerUtils.isOidcAccount(context, account); @@ -426,7 +425,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - OIDCTokenRefresher.Companion.refresh(context, account); + OIDCTokenRefresher.refresh(context, account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); diff --git a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt index 7a5f7d84e..c8f8891b5 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt @@ -36,6 +36,7 @@ import java.util.logging.Level class OIDCTokenRefresher { companion object { + @JvmStatic fun refresh(context: Context, account: Account) { if (!AccountManagerUtils.isOidcAccount(context, account)) { return @@ -43,18 +44,17 @@ class OIDCTokenRefresher { val accountSettings = AccountSettings(context, account) val credentials = accountSettings.credentials() - if (credentials.authState == null) { - return - } + val authState = credentials.authState ?: return - val authorizationService = EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java).authorizationService() + val authorizationService = + EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java) + .authorizationService() val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret) - val updatedAuthState = refreshAuthState(authorizationService, credentials.authState, clientAuth) - if (updatedAuthState == null) { - return - } + val updatedAuthState = + refreshAuthState(authorizationService, authState, clientAuth) ?: return + // update authState into osAccountManager accountSettings.credentials( accountSettings.credentials().copy( authState = updatedAuthState @@ -62,6 +62,7 @@ class OIDCTokenRefresher { ) } + @JvmStatic private fun refreshAuthState( authService: AuthorizationService, authState: AuthState, -- GitLab From 2445c90b19f3c2e1fdc5d462bbadf4f9638f8c16 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 23 Apr 2025 13:39:06 +0600 Subject: [PATCH 05/13] chore: update according to review --- .../com/nextcloud/android/sso/InputStreamBinder.java | 12 ++++++++++-- .../com/nextcloud/android/sso/OIDCTokenRefresher.kt | 6 ------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 9e7fc078a..577747a4a 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -334,7 +334,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - OIDCTokenRefresher.refresh(context, account); + refreshOIDCTokenIfNeeded(account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); @@ -390,6 +390,14 @@ public class InputStreamBinder extends IInputStreamService.Stub { } } + private void refreshOIDCTokenIfNeeded(Account account) { + if (!AccountManagerUtils.isOidcAccount(context, account)) { + return; + } + + OIDCTokenRefresher.refresh(context, account); + } + /* * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. @@ -425,7 +433,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - OIDCTokenRefresher.refresh(context, account); + refreshOIDCTokenIfNeeded(account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); diff --git a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt index c8f8891b5..d608b3116 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt @@ -22,7 +22,6 @@ import at.bitfire.davdroid.OpenIdUtils import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint import at.bitfire.davdroid.settings.AccountSettings -import com.nextcloud.android.utils.AccountManagerUtils import dagger.hilt.android.EntryPointAccessors import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred @@ -38,10 +37,6 @@ class OIDCTokenRefresher { @JvmStatic fun refresh(context: Context, account: Account) { - if (!AccountManagerUtils.isOidcAccount(context, account)) { - return - } - val accountSettings = AccountSettings(context, account) val credentials = accountSettings.credentials() val authState = credentials.authState ?: return @@ -83,7 +78,6 @@ class OIDCTokenRefresher { } } - // return value try { authStateFuture.await() } catch (ignored: CancellationException) { -- GitLab From df7b9be02542d31bc598b5a301f78c67bd90dc8a Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 22 Apr 2025 21:25:52 +0600 Subject: [PATCH 06/13] fix: remove unwanted murena_client_secret value --- .../at/bitfire/davdroid/authorization/IdentityProvider.kt | 2 +- .../at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt b/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt index 02e843794..c7cdbf441 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt @@ -42,7 +42,7 @@ enum class IdentityProvider( authEndpoint = null, tokenEndpoint = null, clientId = BuildConfig.MURENA_CLIENT_ID, - clientSecret = null, + clientSecret = BuildConfig.MURENA_CLIENT_SECRET, redirectUri = BuildConfig.MURENA_REDIRECT_URI + ":/redirect", logoutRedirectUri = BuildConfig.MURENA_LOGOUT_REDIRECT_URI + ":/redirect", scope = "openid profile email offline_access", diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt index 6e785ffc6..9b05b2e60 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt @@ -27,7 +27,7 @@ class EeloAuthenticatorModel(application: Application) : AndroidViewModel(applic companion object { // as https://gitlab.e.foundation/e/backlog/-/issues/6287 is blocked, the openId implementation is not ready yet. // But we want to push the changes so later we won't face any conflict. So we are disabling the openId feature for now. - const val ENABLE_OIDC_SUPPORT = false + const val ENABLE_OIDC_SUPPORT = true } private var initialized = false -- GitLab From 410ee59896ab2ca4d209e97d0f7d21238c069cd3 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 22 Apr 2025 21:27:06 +0600 Subject: [PATCH 07/13] Revert "fix: remove unwanted murena_client_secret value" This reverts commit 56144e295170f07ec223cb0bb21e50f7bde2b5da. --- .../at/bitfire/davdroid/authorization/IdentityProvider.kt | 2 +- .../at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt b/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt index c7cdbf441..02e843794 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt @@ -42,7 +42,7 @@ enum class IdentityProvider( authEndpoint = null, tokenEndpoint = null, clientId = BuildConfig.MURENA_CLIENT_ID, - clientSecret = BuildConfig.MURENA_CLIENT_SECRET, + clientSecret = null, redirectUri = BuildConfig.MURENA_REDIRECT_URI + ":/redirect", logoutRedirectUri = BuildConfig.MURENA_LOGOUT_REDIRECT_URI + ":/redirect", scope = "openid profile email offline_access", diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt index 9b05b2e60..6e785ffc6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt @@ -27,7 +27,7 @@ class EeloAuthenticatorModel(application: Application) : AndroidViewModel(applic companion object { // as https://gitlab.e.foundation/e/backlog/-/issues/6287 is blocked, the openId implementation is not ready yet. // But we want to push the changes so later we won't face any conflict. So we are disabling the openId feature for now. - const val ENABLE_OIDC_SUPPORT = true + const val ENABLE_OIDC_SUPPORT = false } private var initialized = false -- GitLab From a3ba2c852f4bac471a71b36266e358e3e4655064 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Wed, 23 Apr 2025 16:53:53 +0600 Subject: [PATCH 08/13] refactor: use suspendCoroutine to convert callback-based API call - Used suspendCoroutine over CompletableDeferred for less boilerplate and leaner API. - Made the refresh() method call use runBlocking() and using @Blocking annotation to let the consumer of the API inform about the thread safety. - Handled exceptions gracefully. - Added log information when refresh is not needed or not possible. --- .../android/sso/OidcTokenRefresher.kt | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt diff --git a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt new file mode 100644 index 000000000..07f4c109b --- /dev/null +++ b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2025 e Foundation + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package com.nextcloud.android.sso + +import android.accounts.Account +import android.content.Context +import at.bitfire.davdroid.OpenIdUtils +import at.bitfire.davdroid.log.Logger +import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint +import at.bitfire.davdroid.settings.AccountSettings +import com.nextcloud.android.utils.AccountManagerUtils +import dagger.hilt.android.EntryPointAccessors +import kotlinx.coroutines.runBlocking +import net.openid.appauth.AuthState +import net.openid.appauth.AuthorizationService +import net.openid.appauth.ClientAuthentication +import org.jetbrains.annotations.Blocking +import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException +import kotlin.coroutines.suspendCoroutine + +/** + * Utility for refreshing OpenID Connect (OIDC) tokens in the Android AccountManager. + * + * This object exposes a synchronous, blocking entry point for token refresh requests + * and internally uses coroutines to perform the refresh operation with proper + * callback-to-suspension conversion. + */ +object OidcTokenRefresher { + + /** + * Refreshes the OIDC token for the given [Account] if it is an OIDC account. + * + * This method blocks the calling thread until the refresh completes or fails. + * It will: + * 1. Verify that the account is managed by OIDC. + * 2. Load stored credentials and their current [AuthState]. + * 3. Invoke the authorization service to refresh tokens. + * 4. Update the AccountManager on successful refresh or log failures. + * + * **Threading:** This method uses [runBlocking] and therefore must **not** be + * called from the Main/UI thread. It is annotated with `@Blocking` to signal + * blocking behavior. + * + * @param context Android [Context] used to access AccountManager, resources, and + * authorization services. + * @param account The [Account] whose OIDC tokens should be refreshed. + * + * @throws CancellationException if the coroutine is cancelled during execution. + */ + @JvmStatic + @Blocking + fun refresh(context: Context, account: Account) { + runBlocking { + val accountSettings = AccountSettings(context, account) + val credentials = accountSettings.credentials() + val authState = credentials.authState + + if (authState == null) { + Logger.log.info("Account: ${account.name} has null AuthState, ignoring refresh request.") + return@runBlocking + } + + val authorizationService = + EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java) + .authorizationService() + val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret) + + val updatedAuthState = runCatching { + refreshAuthState(authorizationService, authState, clientAuth) + }.getOrNull() + + if (updatedAuthState != null) { + updateAndroidAccountManagerAuthState(accountSettings, updatedAuthState) + } else { + Logger.log.info("Couldn't update AuthState for Account: ${account.name}") + } + } + } + + /** + * Suspends until the authState has fresh tokens from AuthorizationService. + * + * Internally it bridges the callback-based `performActionWithFreshTokens` + * API into a coroutine suspension using [suspendCoroutine]. On success, it + * resumes with the same [AuthState] instance containing updated tokens. On + * failure, it throws the encountered [Throwable]. + * + * @param authService The [AuthorizationService] to use for token refresh. + * @param authState The current [AuthState] containing existing tokens. + * @param clientAuth [ClientAuthentication] mechanism (e.g., client secret). + * @return The same [AuthState] instance with refreshed tokens. + * @throws Exception if the refresh operation fails. + */ + private suspend fun refreshAuthState( + authService: AuthorizationService, authState: AuthState, clientAuth: ClientAuthentication + ): AuthState { + return suspendCoroutine { continuation -> + authState.performActionWithFreshTokens( + authService, + clientAuth + ) { accessToken, _, authorizationException -> + when { + accessToken != null -> continuation.resume(authState) + authorizationException != null -> continuation.resumeWithException( + authorizationException + ) + } + } + } + } + + /** + * Persists an updated [AuthState] back into the Android AccountManager. + */ + private fun updateAndroidAccountManagerAuthState( + accountSettings: AccountSettings, updatedAuthState: AuthState + ) = accountSettings.credentials( + accountSettings.credentials().copy(authState = updatedAuthState) + ) +} -- GitLab From 93afb4fe322f2e06ce8f39ff7acc2c560756a533 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Wed, 23 Apr 2025 17:12:35 +0600 Subject: [PATCH 09/13] refactor: use suspendCoroutine to convert callback-based API call - Used suspendCoroutine over CompletableDeferred for less boilerplate and leaner API. - Made the refresh() method call use runBlocking() and using @Blocking annotation to let the consumer of the API inform about the thread safety. - Handled exceptions gracefully. - Added log information when refresh is not needed or not possible. --- .../android/sso/InputStreamBinder.java | 134 ++++++++++-------- .../android/sso/OIDCTokenRefresher.kt | 89 ------------ .../android/sso/OidcTokenRefresher.kt | 20 ++- 3 files changed, 82 insertions(+), 161 deletions(-) delete mode 100644 app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 577747a4a..102a97f62 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -49,6 +49,8 @@ import com.owncloud.android.lib.common.OwnCloudClientManager; import com.owncloud.android.lib.common.OwnCloudClientManagerFactory; import com.owncloud.android.lib.common.operations.RemoteOperation; +import net.openid.appauth.AuthState; + import org.apache.commons.httpclient.HttpConnection; import org.apache.commons.httpclient.HttpMethodBase; import org.apache.commons.httpclient.HttpState; @@ -80,7 +82,9 @@ import java.util.List; import java.util.Map; import java.util.logging.Level; +import at.bitfire.davdroid.db.Credentials; import at.bitfire.davdroid.log.Logger; +import at.bitfire.davdroid.settings.AccountSettings; public class InputStreamBinder extends IInputStreamService.Stub { private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json"; @@ -97,6 +101,56 @@ public class InputStreamBinder extends IInputStreamService.Stub { this.context = context; } + // Taken from http://codahale.com/a-lesson-in-timing-attacks/ + private static boolean isEqual(byte[] a, byte[] b) { + if (a.length != b.length) { + return false; + } + + int result = 0; + for (int i = 0; i < a.length; i++) { + result |= a[i] ^ b[i]; + } + return result == 0; + } + + private static String inputStreamToString(InputStream inputStream) { + try { + StringBuilder total = new StringBuilder(); + BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); + String line = reader.readLine(); + while (line != null) { + total.append(line).append('\n'); + line = reader.readLine(); + } + return total.toString(); + } catch (Exception e) { + return e.getMessage(); + } + } + + @VisibleForTesting + public static NameValuePair[] convertMapToNVP(Map map) { + NameValuePair[] nvp = new NameValuePair[map.size()]; + int i = 0; + for (String key : map.keySet()) { + nvp[i] = new NameValuePair(key, map.get(key)); + i++; + } + return nvp; + } + + @VisibleForTesting + public static NameValuePair[] convertListToNVP(Collection list) { + NameValuePair[] nvp = new NameValuePair[list.size()]; + int i = 0; + for (QueryParam pair : list) { + nvp[i] = new NameValuePair(pair.key, pair.value); + i++; + } + return nvp; + } + public ParcelFileDescriptor performNextcloudRequestV2(ParcelFileDescriptor input) { return performNextcloudRequestAndBodyStreamV2(input, null); } @@ -222,17 +276,6 @@ public class InputStreamBinder extends IInputStreamService.Stub { return result; } - public class NCPropFindMethod extends PropFindMethod { - NCPropFindMethod(String uri, int propfindType, int depth) throws IOException { - super(uri, propfindType, new DavPropertyNameSet(), depth); - } - - @Override - protected void processResponseBody(HttpState httpState, HttpConnection httpConnection) { - // Do not process the response body here. Instead pass it on to client app. - } - } - private HttpMethodBase buildMethod(NextcloudRequest request, Uri baseUri, InputStream requestBodyInputStream) throws IOException { String requestUrl = baseUri + request.getUrl(); @@ -334,7 +377,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - refreshOIDCTokenIfNeeded(account); + refreshOidcToken(account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); @@ -390,12 +433,22 @@ public class InputStreamBinder extends IInputStreamService.Stub { } } - private void refreshOIDCTokenIfNeeded(Account account) { + private void refreshOidcToken(Account account) { if (!AccountManagerUtils.isOidcAccount(context, account)) { + Logger.INSTANCE.getLog().log(Level.FINE, "Account is not an OIDC account, ignoring token refresh."); return; } - OIDCTokenRefresher.refresh(context, account); + AccountSettings accountSettings = new AccountSettings(context, account); + Credentials credentials = accountSettings.credentials(); + AuthState authState = credentials.getAuthState(); + if (authState == null) { + Logger.INSTANCE.getLog().log(Level.FINE, "Account has null AuthState, ignoring token refresh."); + return; + } + + // Blocking call + OidcTokenRefresher.refresh(context, accountSettings, credentials, authState); } /* @@ -433,7 +486,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - refreshOIDCTokenIfNeeded(account); + refreshOidcToken(account); final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); @@ -511,53 +564,14 @@ public class InputStreamBinder extends IInputStreamService.Stub { return isEqual(hash.getBytes(), newHash.getBytes()); } - // Taken from http://codahale.com/a-lesson-in-timing-attacks/ - private static boolean isEqual(byte[] a, byte[] b) { - if (a.length != b.length) { - return false; - } - - int result = 0; - for (int i = 0; i < a.length; i++) { - result |= a[i] ^ b[i]; - } - return result == 0; - } - - private static String inputStreamToString(InputStream inputStream) { - try { - StringBuilder total = new StringBuilder(); - BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); - String line = reader.readLine(); - while (line != null) { - total.append(line).append('\n'); - line = reader.readLine(); - } - return total.toString(); - } catch (Exception e) { - return e.getMessage(); - } - } - - @VisibleForTesting - public static NameValuePair[] convertMapToNVP(Map map) { - NameValuePair[] nvp = new NameValuePair[map.size()]; - int i = 0; - for (String key : map.keySet()) { - nvp[i] = new NameValuePair(key, map.get(key)); - i++; + public class NCPropFindMethod extends PropFindMethod { + NCPropFindMethod(String uri, int propfindType, int depth) throws IOException { + super(uri, propfindType, new DavPropertyNameSet(), depth); } - return nvp; - } - @VisibleForTesting - public static NameValuePair[] convertListToNVP(Collection list) { - NameValuePair[] nvp = new NameValuePair[list.size()]; - int i = 0; - for (QueryParam pair : list) { - nvp[i] = new NameValuePair(pair.key, pair.value); - i++; + @Override + protected void processResponseBody(HttpState httpState, HttpConnection httpConnection) { + // Do not process the response body here. Instead pass it on to client app. } - return nvp; } } diff --git a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt deleted file mode 100644 index d608b3116..000000000 --- a/app/src/main/java/com/nextcloud/android/sso/OIDCTokenRefresher.kt +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright MURENA SAS 2025 - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.nextcloud.android.sso - -import android.accounts.Account -import android.content.Context -import at.bitfire.davdroid.OpenIdUtils -import at.bitfire.davdroid.log.Logger -import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint -import at.bitfire.davdroid.settings.AccountSettings -import dagger.hilt.android.EntryPointAccessors -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.runBlocking -import net.openid.appauth.AuthState -import net.openid.appauth.AuthorizationException -import net.openid.appauth.AuthorizationService -import net.openid.appauth.ClientAuthentication -import java.util.logging.Level - -class OIDCTokenRefresher { - companion object { - - @JvmStatic - fun refresh(context: Context, account: Account) { - val accountSettings = AccountSettings(context, account) - val credentials = accountSettings.credentials() - val authState = credentials.authState ?: return - - val authorizationService = - EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java) - .authorizationService() - val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret) - - val updatedAuthState = - refreshAuthState(authorizationService, authState, clientAuth) ?: return - - // update authState into osAccountManager - accountSettings.credentials( - accountSettings.credentials().copy( - authState = updatedAuthState - ) - ) - } - - @JvmStatic - private fun refreshAuthState( - authService: AuthorizationService, - authState: AuthState, - clientAuth: ClientAuthentication - ): AuthState? { - return runBlocking { - val authStateFuture = CompletableDeferred() - - authState.performActionWithFreshTokens( - authService, - clientAuth - ) { accessToken: String?, _: String?, ex: AuthorizationException? -> - if (accessToken != null) { - authStateFuture.complete(authState) - } else { - Logger.log.log(Level.WARNING, "Couldn't refresh access token", ex) - authStateFuture.cancel() - } - } - - try { - authStateFuture.await() - } catch (ignored: CancellationException) { - null - } - } - } - } -} diff --git a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt index 07f4c109b..cb6b1ce22 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt @@ -21,10 +21,10 @@ package com.nextcloud.android.sso import android.accounts.Account import android.content.Context import at.bitfire.davdroid.OpenIdUtils +import at.bitfire.davdroid.db.Credentials import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint import at.bitfire.davdroid.settings.AccountSettings -import com.nextcloud.android.utils.AccountManagerUtils import dagger.hilt.android.EntryPointAccessors import kotlinx.coroutines.runBlocking import net.openid.appauth.AuthState @@ -66,17 +66,13 @@ object OidcTokenRefresher { */ @JvmStatic @Blocking - fun refresh(context: Context, account: Account) { + fun refresh( + context: Context, + accountSettings: AccountSettings, + credentials: Credentials, + authState: AuthState, + ) { runBlocking { - val accountSettings = AccountSettings(context, account) - val credentials = accountSettings.credentials() - val authState = credentials.authState - - if (authState == null) { - Logger.log.info("Account: ${account.name} has null AuthState, ignoring refresh request.") - return@runBlocking - } - val authorizationService = EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java) .authorizationService() @@ -89,7 +85,7 @@ object OidcTokenRefresher { if (updatedAuthState != null) { updateAndroidAccountManagerAuthState(accountSettings, updatedAuthState) } else { - Logger.log.info("Couldn't update AuthState for Account: ${account.name}") + Logger.log.warning("Couldn't update AuthState for account.") } } } -- GitLab From c6e31fdb5c9c1f109f995963173a2942e3611ffd Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Wed, 23 Apr 2025 17:22:39 +0600 Subject: [PATCH 10/13] refactor: revert unneeded changes --- .../android/sso/InputStreamBinder.java | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 102a97f62..2e89135de 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -101,56 +101,6 @@ public class InputStreamBinder extends IInputStreamService.Stub { this.context = context; } - // Taken from http://codahale.com/a-lesson-in-timing-attacks/ - private static boolean isEqual(byte[] a, byte[] b) { - if (a.length != b.length) { - return false; - } - - int result = 0; - for (int i = 0; i < a.length; i++) { - result |= a[i] ^ b[i]; - } - return result == 0; - } - - private static String inputStreamToString(InputStream inputStream) { - try { - StringBuilder total = new StringBuilder(); - BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); - String line = reader.readLine(); - while (line != null) { - total.append(line).append('\n'); - line = reader.readLine(); - } - return total.toString(); - } catch (Exception e) { - return e.getMessage(); - } - } - - @VisibleForTesting - public static NameValuePair[] convertMapToNVP(Map map) { - NameValuePair[] nvp = new NameValuePair[map.size()]; - int i = 0; - for (String key : map.keySet()) { - nvp[i] = new NameValuePair(key, map.get(key)); - i++; - } - return nvp; - } - - @VisibleForTesting - public static NameValuePair[] convertListToNVP(Collection list) { - NameValuePair[] nvp = new NameValuePair[list.size()]; - int i = 0; - for (QueryParam pair : list) { - nvp[i] = new NameValuePair(pair.key, pair.value); - i++; - } - return nvp; - } - public ParcelFileDescriptor performNextcloudRequestV2(ParcelFileDescriptor input) { return performNextcloudRequestAndBodyStreamV2(input, null); } @@ -276,6 +226,17 @@ public class InputStreamBinder extends IInputStreamService.Stub { return result; } + public class NCPropFindMethod extends PropFindMethod { + NCPropFindMethod(String uri, int propfindType, int depth) throws IOException { + super(uri, propfindType, new DavPropertyNameSet(), depth); + } + + @Override + protected void processResponseBody(HttpState httpState, HttpConnection httpConnection) { + // Do not process the response body here. Instead pass it on to client app. + } + } + private HttpMethodBase buildMethod(NextcloudRequest request, Uri baseUri, InputStream requestBodyInputStream) throws IOException { String requestUrl = baseUri + request.getUrl(); @@ -452,10 +413,10 @@ public class InputStreamBinder extends IInputStreamService.Stub { } /* - * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. - * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. - * - * These nextcloud app request paths contain `/index.php/apps/` on them. + * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. + * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. + * + * These nextcloud app request paths contain `/index.php/apps/` on them. */ private boolean shouldAddHeaderForOidcLogin(@NonNull Context context, @NonNull Account account, @NonNull String path) { boolean isOidcAccount = AccountManagerUtils.isOidcAccount(context, account); @@ -564,14 +525,53 @@ public class InputStreamBinder extends IInputStreamService.Stub { return isEqual(hash.getBytes(), newHash.getBytes()); } - public class NCPropFindMethod extends PropFindMethod { - NCPropFindMethod(String uri, int propfindType, int depth) throws IOException { - super(uri, propfindType, new DavPropertyNameSet(), depth); + // Taken from http://codahale.com/a-lesson-in-timing-attacks/ + private static boolean isEqual(byte[] a, byte[] b) { + if (a.length != b.length) { + return false; } - @Override - protected void processResponseBody(HttpState httpState, HttpConnection httpConnection) { - // Do not process the response body here. Instead pass it on to client app. + int result = 0; + for (int i = 0; i < a.length; i++) { + result |= a[i] ^ b[i]; + } + return result == 0; + } + + private static String inputStreamToString(InputStream inputStream) { + try { + StringBuilder total = new StringBuilder(); + BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); + String line = reader.readLine(); + while (line != null) { + total.append(line).append('\n'); + line = reader.readLine(); + } + return total.toString(); + } catch (Exception e) { + return e.getMessage(); + } + } + + @VisibleForTesting + public static NameValuePair[] convertMapToNVP(Map map) { + NameValuePair[] nvp = new NameValuePair[map.size()]; + int i = 0; + for (String key : map.keySet()) { + nvp[i] = new NameValuePair(key, map.get(key)); + i++; + } + return nvp; + } + + @VisibleForTesting + public static NameValuePair[] convertListToNVP(Collection list) { + NameValuePair[] nvp = new NameValuePair[list.size()]; + int i = 0; + for (QueryParam pair : list) { + nvp[i] = new NameValuePair(pair.key, pair.value); + i++; } + return nvp; } } -- GitLab From 28ae5a8f951d4f2a451d3ab3cb3af2d1f25bac9a Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Wed, 23 Apr 2025 19:29:37 +0600 Subject: [PATCH 11/13] refactor: improve calling of refreshToken() method inside InputStreamBinder's methods --- .../android/sso/InputStreamBinder.java | 20 +++++------ .../android/sso/OidcTokenRefresher.kt | 36 ++++++++++++------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 2e89135de..8587387ef 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -338,7 +338,9 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - refreshOidcToken(account); + if (OidcTokenRefresher.isRequired(context, account)) { + refreshToken(account); + } final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); @@ -394,19 +396,11 @@ public class InputStreamBinder extends IInputStreamService.Stub { } } - private void refreshOidcToken(Account account) { - if (!AccountManagerUtils.isOidcAccount(context, account)) { - Logger.INSTANCE.getLog().log(Level.FINE, "Account is not an OIDC account, ignoring token refresh."); - return; - } - + private void refreshToken(Account account) { AccountSettings accountSettings = new AccountSettings(context, account); Credentials credentials = accountSettings.credentials(); AuthState authState = credentials.getAuthState(); - if (authState == null) { - Logger.INSTANCE.getLog().log(Level.FINE, "Account has null AuthState, ignoring token refresh."); - return; - } + assert authState != null; // Blocking call OidcTokenRefresher.refresh(context, accountSettings, credentials, authState); @@ -447,7 +441,9 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - refreshOidcToken(account); + if (OidcTokenRefresher.isRequired(context, account)) { + refreshToken(account); + } final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); final OwnCloudAccount ownCloudAccount = new OwnCloudAccount(account, context); diff --git a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt index cb6b1ce22..e87ab509d 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt @@ -25,12 +25,14 @@ import at.bitfire.davdroid.db.Credentials import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint import at.bitfire.davdroid.settings.AccountSettings +import com.nextcloud.android.utils.AccountManagerUtils import dagger.hilt.android.EntryPointAccessors import kotlinx.coroutines.runBlocking import net.openid.appauth.AuthState import net.openid.appauth.AuthorizationService import net.openid.appauth.ClientAuthentication import org.jetbrains.annotations.Blocking +import java.util.logging.Level import kotlin.coroutines.resume import kotlin.coroutines.resumeWithException import kotlin.coroutines.suspendCoroutine @@ -45,24 +47,15 @@ import kotlin.coroutines.suspendCoroutine object OidcTokenRefresher { /** - * Refreshes the OIDC token for the given [Account] if it is an OIDC account. + * Refreshes the OIDC token for the given [Account]. * - * This method blocks the calling thread until the refresh completes or fails. * It will: - * 1. Verify that the account is managed by OIDC. - * 2. Load stored credentials and their current [AuthState]. - * 3. Invoke the authorization service to refresh tokens. - * 4. Update the AccountManager on successful refresh or log failures. + * 1. Invoke the authorization service to refresh tokens. + * 2. Update AccountManager on successful refresh or log failures. * * **Threading:** This method uses [runBlocking] and therefore must **not** be * called from the Main/UI thread. It is annotated with `@Blocking` to signal * blocking behavior. - * - * @param context Android [Context] used to access AccountManager, resources, and - * authorization services. - * @param account The [Account] whose OIDC tokens should be refreshed. - * - * @throws CancellationException if the coroutine is cancelled during execution. */ @JvmStatic @Blocking @@ -90,6 +83,25 @@ object OidcTokenRefresher { } } + @JvmStatic + fun isRequired(context: Context, account: Account): Boolean { + if (!AccountManagerUtils.isOidcAccount(context, account)) { + Logger.log.log(Level.FINE, "Account is not an OIDC account, refresh isn't required.") + return false + } + + val accountSettings = AccountSettings(context, account) + val credentials = accountSettings.credentials() + val authState = credentials.authState + + if (authState == null) { + Logger.log.log(Level.FINE, "Account has null AuthState, refresh isn't required.") + return false + } + + return true + } + /** * Suspends until the authState has fresh tokens from AuthorizationService. * -- GitLab From 4c55c08c710be7c35947035f605589c40c653059 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Thu, 24 Apr 2025 13:57:03 +0600 Subject: [PATCH 12/13] refactor: improve refresh() invocation and validation before request --- .../android/sso/InputStreamBinder.java | 16 +++----- .../android/sso/OidcTokenRefresher.kt | 39 ++++++------------- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 8587387ef..44ccc979a 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -338,8 +338,9 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - if (OidcTokenRefresher.isRequired(context, account)) { - refreshToken(account); + if (AccountManagerUtils.isOidcAccount(context, account)) { + // Blocking call + OidcTokenRefresher.refresh(context, account); } final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); @@ -397,13 +398,7 @@ public class InputStreamBinder extends IInputStreamService.Stub { } private void refreshToken(Account account) { - AccountSettings accountSettings = new AccountSettings(context, account); - Credentials credentials = accountSettings.credentials(); - AuthState authState = credentials.getAuthState(); - assert authState != null; - // Blocking call - OidcTokenRefresher.refresh(context, accountSettings, credentials, authState); } /* @@ -441,8 +436,9 @@ public class InputStreamBinder extends IInputStreamService.Stub { new IllegalStateException("URL need to start with a /")); } - if (OidcTokenRefresher.isRequired(context, account)) { - refreshToken(account); + if (AccountManagerUtils.isOidcAccount(context, account)) { + // Blocking call + OidcTokenRefresher.refresh(context, account); } final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton(); diff --git a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt index e87ab509d..3b180bb91 100644 --- a/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt +++ b/app/src/main/java/com/nextcloud/android/sso/OidcTokenRefresher.kt @@ -21,11 +21,9 @@ package com.nextcloud.android.sso import android.accounts.Account import android.content.Context import at.bitfire.davdroid.OpenIdUtils -import at.bitfire.davdroid.db.Credentials import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint import at.bitfire.davdroid.settings.AccountSettings -import com.nextcloud.android.utils.AccountManagerUtils import dagger.hilt.android.EntryPointAccessors import kotlinx.coroutines.runBlocking import net.openid.appauth.AuthState @@ -59,13 +57,17 @@ object OidcTokenRefresher { */ @JvmStatic @Blocking - fun refresh( - context: Context, - accountSettings: AccountSettings, - credentials: Credentials, - authState: AuthState, - ) { + fun refresh(context: Context, account: Account) { runBlocking { + val accountSettings = AccountSettings(context, account) + val credentials = accountSettings.credentials() + val authState = credentials.authState + + if (authState == null) { + Logger.log.log(Level.FINE, "Account: $account has null AuthState, refresh isn't possible.") + return@runBlocking + } + val authorizationService = EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java) .authorizationService() @@ -78,30 +80,11 @@ object OidcTokenRefresher { if (updatedAuthState != null) { updateAndroidAccountManagerAuthState(accountSettings, updatedAuthState) } else { - Logger.log.warning("Couldn't update AuthState for account.") + Logger.log.warning("Couldn't update AuthState for account: $account") } } } - @JvmStatic - fun isRequired(context: Context, account: Account): Boolean { - if (!AccountManagerUtils.isOidcAccount(context, account)) { - Logger.log.log(Level.FINE, "Account is not an OIDC account, refresh isn't required.") - return false - } - - val accountSettings = AccountSettings(context, account) - val credentials = accountSettings.credentials() - val authState = credentials.authState - - if (authState == null) { - Logger.log.log(Level.FINE, "Account has null AuthState, refresh isn't required.") - return false - } - - return true - } - /** * Suspends until the authState has fresh tokens from AuthorizationService. * -- GitLab From 46014aad8f41c94f52b812e967bff5bd5df57898 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Thu, 24 Apr 2025 16:14:13 +0600 Subject: [PATCH 13/13] refactor: remove unused code --- .../java/com/nextcloud/android/sso/InputStreamBinder.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java index 44ccc979a..5e7b80e3e 100644 --- a/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java +++ b/app/src/main/java/com/nextcloud/android/sso/InputStreamBinder.java @@ -49,8 +49,6 @@ import com.owncloud.android.lib.common.OwnCloudClientManager; import com.owncloud.android.lib.common.OwnCloudClientManagerFactory; import com.owncloud.android.lib.common.operations.RemoteOperation; -import net.openid.appauth.AuthState; - import org.apache.commons.httpclient.HttpConnection; import org.apache.commons.httpclient.HttpMethodBase; import org.apache.commons.httpclient.HttpState; @@ -82,9 +80,7 @@ import java.util.List; import java.util.Map; import java.util.logging.Level; -import at.bitfire.davdroid.db.Credentials; import at.bitfire.davdroid.log.Logger; -import at.bitfire.davdroid.settings.AccountSettings; public class InputStreamBinder extends IInputStreamService.Stub { private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json"; @@ -397,10 +393,6 @@ public class InputStreamBinder extends IInputStreamService.Stub { } } - private void refreshToken(Account account) { - - } - /* * for non ocs/dav requests (nextcloud app: ex: notes app), when OIDC is used, we need to pass an special header. * We should not pass this header for ocs/dav requests as it can cause session cookie not being used for those request. -- GitLab