Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 78155d10 authored by Fahim M. Choudhury's avatar Fahim M. Choudhury
Browse files

Merge branch '3559-fix-random-auth-failure-oauth' into 'main'

fix: synchronize token refresh to mitigate race condition

See merge request !181
parents 74ce6e1d 7dee7d26
Loading
Loading
Loading
Loading
Loading
+47 −11
Original line number Diff line number Diff line
@@ -24,8 +24,10 @@ import static com.nextcloud.android.sso.Constants.EXCEPTION_INVALID_REQUEST_URL;
import static com.nextcloud.android.sso.Constants.EXCEPTION_INVALID_TOKEN;
import static com.nextcloud.android.sso.Constants.EXCEPTION_UNSUPPORTED_METHOD;
import static com.nextcloud.android.sso.Constants.SSO_SHARED_PREFERENCE;
import static at.bitfire.davdroid.settings.AccountSettings.KEY_AUTH_STATE;

import android.accounts.Account;
import android.accounts.AccountManager;
import android.accounts.AuthenticatorException;
import android.accounts.OperationCanceledException;
import android.content.Context;
@@ -47,8 +49,13 @@ import com.owncloud.android.lib.common.OwnCloudAccount;
import com.owncloud.android.lib.common.OwnCloudClient;
import com.owncloud.android.lib.common.OwnCloudClientManager;
import com.owncloud.android.lib.common.OwnCloudClientManagerFactory;
import com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundException;
import com.owncloud.android.lib.common.operations.RemoteOperation;

import net.openid.appauth.AuthState;
import net.openid.appauth.AuthorizationException;
import net.openid.appauth.ClientAuthentication;

import org.apache.commons.httpclient.HttpConnection;
import org.apache.commons.httpclient.HttpMethodBase;
import org.apache.commons.httpclient.HttpState;
@@ -80,7 +87,15 @@ import java.util.List;
import java.util.Map;
import java.util.logging.Level;

import at.bitfire.davdroid.OpenIdUtils;
import at.bitfire.davdroid.db.Credentials;
import at.bitfire.davdroid.log.Logger;
import at.bitfire.davdroid.settings.AccountSettings;
import at.bitfire.davdroid.util.AuthStatePrefUtils;
import at.bitfire.davdroid.util.CompatUtilsKt;
import kotlin.Unit;
import kotlin.jvm.functions.Function0;
import kotlin.jvm.functions.Function1;

public class InputStreamBinder extends IInputStreamService.Stub {
    private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json";
@@ -315,10 +330,9 @@ public class InputStreamBinder extends IInputStreamService.Stub {
        return method;
    }

    private HttpMethodBase processRequest(final NextcloudRequest request, final InputStream requestBodyInputStream)
            throws UnsupportedOperationException,
            com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundException,
            OperationCanceledException, AuthenticatorException, IOException {
    private HttpMethodBase processRequest(final NextcloudRequest request, final InputStream requestBodyInputStream) throws
            UnsupportedOperationException, AccountNotFoundException, OperationCanceledException,
            AuthenticatorException, IOException, AuthorizationException {
        Account account = AccountManagerUtils.getAccountByName(context, request.getAccountName());
        if (account == null) {
            throw new IllegalStateException(EXCEPTION_ACCOUNT_NOT_FOUND);
@@ -335,8 +349,7 @@ public class InputStreamBinder extends IInputStreamService.Stub {
        }

        if (AccountManagerUtils.isOidcAccount(context, account)) {
            // Blocking call
            OidcTokenRefresher.refresh(context, account);
            refreshAuth(account);
        }

        final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton();
@@ -409,9 +422,8 @@ public class InputStreamBinder extends IInputStreamService.Stub {
    }

    private Response processRequestV2(final NextcloudRequest request, final InputStream requestBodyInputStream)
            throws UnsupportedOperationException,
            com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundException,
            OperationCanceledException, AuthenticatorException, IOException {
            throws UnsupportedOperationException, AccountNotFoundException, OperationCanceledException,
            AuthenticatorException, IOException, AuthorizationException {
        Account account = AccountManagerUtils.getAccountByName(context, request.getAccountName());
        if (account == null) {
            throw new IllegalStateException(EXCEPTION_ACCOUNT_NOT_FOUND);
@@ -429,8 +441,7 @@ public class InputStreamBinder extends IInputStreamService.Stub {
        }

        if (AccountManagerUtils.isOidcAccount(context, account)) {
            // Blocking call
            OidcTokenRefresher.refresh(context, account);
            refreshAuth(account);
        }

        final OwnCloudClientManager ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton();
@@ -486,6 +497,31 @@ public class InputStreamBinder extends IInputStreamService.Stub {
        }
    }

    private void refreshAuth(@NonNull Account account) throws AuthorizationException {
        AccountSettings accountSettings;
        try {
            accountSettings = new AccountSettings(context, account);
        } catch (IllegalArgumentException e) {
            throw new IllegalStateException(e.getMessage());
        }

        Credentials credentials = accountSettings.credentials();
        Function0<ClientAuthentication> getClientAuth =
                () -> OpenIdUtils.getClientAuthentication(credentials.getClientSecret());

        Function0<AuthState> readAuthState = credentials::getAuthState;

        Function1<AuthState, Unit> writeAuthState = authState -> {
            AccountManager accountManager = AccountManager.get(context);
            CompatUtilsKt.setAndVerifyUserData(accountManager, account, KEY_AUTH_STATE, authState.jsonSerializeString());
            AuthStatePrefUtils.saveAuthState(context, account, authState.jsonSerializeString());
            return Unit.INSTANCE;
        };

        // Blocking call
        OidcTokenRefresher.refreshAuthState(context, account, getClientAuth, readAuthState, writeAuthState);
    }

    private boolean isValid(NextcloudRequest request) {
        String callingPackageName = context.getPackageManager().getNameForUid(Binder.getCallingUid());

+71 −70
Original line number Diff line number Diff line
@@ -20,109 +20,110 @@ 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.BuildConfig
import at.bitfire.davdroid.network.HttpClient.HttpClientEntryPoint
import at.bitfire.davdroid.settings.AccountSettings
import dagger.hilt.android.EntryPointAccessors
import kotlinx.coroutines.runBlocking
import net.openid.appauth.AuthState
import net.openid.appauth.AuthorizationService
import net.openid.appauth.AuthorizationException
import net.openid.appauth.ClientAuthentication
import org.jetbrains.annotations.Blocking
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionException
import java.util.logging.Level
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
import kotlin.coroutines.suspendCoroutine
import java.util.logging.Logger

/**
 * 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.
 * Utility for refreshing OAuth tokens and updating them in the Android AccountManager.
 */
object OidcTokenRefresher {
    private val logger: Logger = Logger.getGlobal()

    /**
     * Refreshes the OIDC token for the given [Account].
     * Refreshes the current AuthState and updates it. Uses the current one if it's still valid,
     * or requests a new one if necessary.
     *
     * It will:
     * 1. Invoke the authorization service to refresh tokens.
     * 1. Invoke the AppAuth library's 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.
     *
     * This method is synchronized / thread-safe so that it can be called for
     * multiple HTTP requests at the same time.
     *
     * Returns an updated AuthState if token refresh is successful;
     * Throws [AuthorizationException.TokenRequestErrors.INVALID_GRANT] for invalid grant or null otherwise.
     */

    @JvmStatic
    @Blocking
    fun refresh(context: Context, account: Account) {
        runBlocking {
            val accountSettings = AccountSettings(context, account)
            val credentials = accountSettings.credentials()
            val authState = credentials.authState
    @Throws(AuthorizationException::class)
    fun refreshAuthState(
        context: Context,
        account: Account?,
        getClientAuth: () -> ClientAuthentication?,
        readAuthState: () -> AuthState?,
        writeAuthState: (AuthState) -> Unit
    ): AuthState? = synchronized(javaClass) {
        val authState = readAuthState() ?: return null
        // Use cached authState if possible
        if (authState.isAuthorized && authState.accessToken != null && !authState.needsTokenRefresh) {
            if (BuildConfig.DEBUG) {
                logger.finest("$account is using cached AuthState: ${authState.jsonSerializeString()}")
            }
            return authState
        }

            if (authState == null) {
                Logger.log.log(Level.FINE, "Account: $account has null AuthState, refresh isn't possible.")
                return@runBlocking
        // Check for AuthorizationException
        val authorizationException = authState.authorizationException
        if (authorizationException != null && isInvalidGrant(authorizationException)) {
            throw AuthorizationException.TokenRequestErrors.INVALID_GRANT
        }

            val authorizationService =
        logger.info("$account is requesting fresh access token")
        if (BuildConfig.DEBUG) {
            logger.finest("AuthState before update = ${authState.jsonSerializeString()}")
        }
        val clientAuth = getClientAuth() ?: return null
        val authService =
            EntryPointAccessors.fromApplication(context, HttpClientEntryPoint::class.java)
                .authorizationService()
            val clientAuth = OpenIdUtils.getClientAuthentication(credentials.clientSecret)

            val updatedAuthState = runCatching {
                refreshAuthState(authorizationService, authState, clientAuth)
            }.getOrNull()
        val authStateFuture = CompletableFuture<AuthState>()

            if (updatedAuthState != null) {
                updateAndroidAccountManagerAuthState(accountSettings, updatedAuthState)
            } else {
                Logger.log.warning("Couldn't update AuthState for account: $account")
            }
        return@synchronized try {
            authState.performActionWithFreshTokens(
                authService, clientAuth
            ) { accessToken, _, exception ->
                writeAuthState(authState)
                when {
                    accessToken != null -> {
                        logger.info("Token refreshed for $account")
                        if (BuildConfig.DEBUG) {
                            logger.finest("Updated authState = ${authState.jsonSerializeString()}")
                        }
                        authStateFuture.complete(authState)
                    }

    /**
     * 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
                    )
                    exception != null -> {
                        authStateFuture.completeExceptionally(exception)
                    }
                }
            }
            authStateFuture.join()
        } catch (e: CompletionException) {
            logger.log(Level.SEVERE, "Couldn't obtain access token", e)
            null
        } finally {
            authService.dispose()
        }
    }

    /**
     * Persists an updated [AuthState] back into the Android AccountManager.
     */
    private fun updateAndroidAccountManagerAuthState(
        accountSettings: AccountSettings, updatedAuthState: AuthState
    ) = accountSettings.credentials(
        accountSettings.credentials().copy(authState = updatedAuthState)
    )
    // Checks whether the given AuthorizationException indicates an invalid grant (requires re-login).
    private fun isInvalidGrant(ex: AuthorizationException?): Boolean {
        val invalidGrant = AuthorizationException.TokenRequestErrors.INVALID_GRANT
        return ex?.code == invalidGrant.code && ex.error == invalidGrant.error
    }
}
+0 −2
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import android.content.Context;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import at.bitfire.davdroid.Constants;
import at.bitfire.davdroid.R;
import at.bitfire.davdroid.settings.AccountSettings;

@@ -61,5 +60,4 @@ public final class AccountManagerUtils {
    public static String getAccountType(@NonNull Context context) {
        return context.getString(R.string.eelo_account_type);
    }

}
+1 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import net.openid.appauth.NoClientAuthentication

object OpenIdUtils {

    @JvmStatic
    fun getClientAuthentication(secret: String?): ClientAuthentication {
        if (secret == null) {
            return NoClientAuthentication.INSTANCE
+32 −40
Original line number Diff line number Diff line
@@ -4,59 +4,52 @@

package at.bitfire.davdroid.network

import at.bitfire.davdroid.log.Logger
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CompletableDeferred
import android.accounts.Account
import android.content.Context
import com.nextcloud.android.sso.OidcTokenRefresher
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 okhttp3.Interceptor
import okhttp3.Response
import java.util.logging.Level
import org.jetbrains.annotations.Blocking
import java.util.logging.Logger

/**
 * Sends an OAuth Bearer token authorization as described in RFC 6750.
 */
class BearerAuthInterceptor(
    private val accessToken: String
): Interceptor {

class BearerAuthInterceptor(private val accessToken: String) : Interceptor {
    companion object {

        fun fromAuthState(authService: AuthorizationService, authState: AuthState, clientAuth: ClientAuthentication,
                          callback: AuthStateUpdateCallback? = null): BearerAuthInterceptor? {
            return runBlocking {
                val accessTokenFuture = CompletableDeferred<String>()

                authState.performActionWithFreshTokens(authService, clientAuth) { accessToken: String?, _: String?, ex: AuthorizationException? ->
                    if (accessToken != null) {
                        // persist updated AuthState
                        callback?.onUpdate(authState)

                        // emit access token
                        accessTokenFuture.complete(accessToken)
        val logger: Logger
            get() = Logger.getGlobal()

        @Blocking
        fun createFromAuthState(
            context: Context,
            account: Account?,
            authState: AuthState,
            clientAuthentication: ClientAuthentication,
            callback: AuthStateUpdateCallback? = null
        ): BearerAuthInterceptor? = runBlocking {
            val updatedAuthState = OidcTokenRefresher.refreshAuthState(
                context,
                account,
                getClientAuth = { clientAuthentication },
                readAuthState = { authState },
                writeAuthState = { callback?.onUpdate(it) }
            )
            val accessToken = updatedAuthState?.accessToken
            if (updatedAuthState != null && accessToken != null) {
                // Persist authState
                callback?.onUpdate(updatedAuthState)
                return@runBlocking BearerAuthInterceptor(accessToken)
            } else {
                return@runBlocking null
            }
                    else {
                        Logger.log.log(Level.WARNING, "Couldn't obtain access token", ex)
                        accessTokenFuture.cancel()
        }
    }

                // return value
                try {
                    BearerAuthInterceptor(accessTokenFuture.await())
                } catch (ignored: CancellationException) {
                    null
                }
            }
        }

    }

    override fun intercept(chain: Interceptor.Chain): Response {
        Logger.log.finer("Authenticating request with access token")
        val rq = chain.request().newBuilder()
            .header("Authorization", "Bearer $accessToken")
            .build()
@@ -67,5 +60,4 @@ class BearerAuthInterceptor(
    fun interface AuthStateUpdateCallback {
        fun onUpdate(authState: AuthState)
    }

}
Loading