fix: persist the registered OAuth client authentication method

Before dynamic registration, the app selects one method from discovery:
client_secret_basic when that method is advertised or when the list is
omitted, and client_secret_post only when basic is absent. A list that
contains neither method stops registration with an error and does not
send a request. The selected method is carried on the registration
result, saved on the account as token_endpoint_auth_method, and reused
for the authorization-code exchange and for refresh. A previously
registered client with no stored method stays on Basic, which is how it
was registered. Clients that were never dynamically registered still use
client_secret_post when discovery advertises it.

Fixes #4891

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

Author: mvanhorn

owncloudApp/src/main/java/com/owncloud/android/presentation/authentication/AccountAuthenticator.java

@@ -54,6 +54,7 @@
 import static com.owncloud.android.data.authentication.AuthenticationConstantsKt.KEY_CLIENT_REGISTRATION_CLIENT_EXPIRATION_DATE;
 import static com.owncloud.android.data.authentication.AuthenticationConstantsKt.KEY_CLIENT_REGISTRATION_CLIENT_ID;
 import static com.owncloud.android.data.authentication.AuthenticationConstantsKt.KEY_CLIENT_REGISTRATION_CLIENT_SECRET;
+import static com.owncloud.android.data.authentication.AuthenticationConstantsKt.KEY_CLIENT_REGISTRATION_TOKEN_ENDPOINT_AUTH_METHOD;
 import static com.owncloud.android.data.authentication.AuthenticationConstantsKt.KEY_OAUTH2_REFRESH_TOKEN;
 import static com.owncloud.android.presentation.authentication.AuthenticatorConstants.KEY_AUTH_TOKEN_TYPE;
 import static org.koin.java.KoinJavaComponent.inject;
@@ -339,10 +340,14 @@ private String refreshToken(
         UseCaseResult<OIDCServerConfiguration> oidcServerConfigurationUseCaseResult =
                 oidcDiscoveryUseCase.getValue().invoke(oidcDiscoveryUseCaseParams);
 
-        String tokenEndpoint;
+        // Capture the dynamic client before resource defaults replace a missing id or secret.
+        String registeredClientId = accountManager.getUserData(account, KEY_CLIENT_REGISTRATION_CLIENT_ID);
+        String registeredClientSecret = accountManager.getUserData(account, KEY_CLIENT_REGISTRATION_CLIENT_SECRET);
+        String storedAuthMethod = accountManager.getUserData(account, KEY_CLIENT_REGISTRATION_TOKEN_ENDPOINT_AUTH_METHOD);
+        boolean hasDynamicClient = registeredClientId != null;
 
-        String clientId = accountManager.getUserData(account, KEY_CLIENT_REGISTRATION_CLIENT_ID);
-        String clientSecret = accountManager.getUserData(account, KEY_CLIENT_REGISTRATION_CLIENT_SECRET);
+        String clientId = registeredClientId;
+        String clientSecret = registeredClientSecret;
 
         String clientIdForRequest = null;
         String clientSecretForRequest = null;
@@ -367,26 +372,38 @@ private String refreshToken(
             }
         }
 
+        String tokenEndpoint;
         if (oidcServerConfigurationUseCaseResult.isSuccess()) {
             Timber.d("OIDC Discovery success. Server discovery info: [ %s ]",
                     oidcServerConfigurationUseCaseResult.getDataOrNull());
 
             // Use token endpoint retrieved from oidc discovery
             tokenEndpoint = oidcServerConfigurationUseCaseResult.getDataOrNull().getTokenEndpoint();
-
-            if (oidcServerConfigurationUseCaseResult.getDataOrNull() != null &&
-            oidcServerConfigurationUseCaseResult.getDataOrNull().isTokenEndpointAuthMethodSupportedClientSecretPost()) {
-                clientIdForRequest = clientId;
-                clientSecretForRequest = clientSecret;
-                useAuthorizationHeader = false;
-            }
         } else {
             Timber.d("OIDC Discovery failed. Server discovery info: [ %s ]",
                     oidcServerConfigurationUseCaseResult.getThrowableOrNull().toString());
 
             tokenEndpoint = baseUrl + File.separator + mContext.getString(R.string.oauth2_url_endpoint_access);
         }
 
+        if (hasDynamicClient) {
+            // Pin the method stored at registration. Legacy dynamic clients have no method and stay on Basic.
+            String authMethod = storedAuthMethod != null
+                    ? storedAuthMethod
+                    : OIDCServerConfiguration.CLIENT_SECRET_BASIC;
+            if (OIDCServerConfiguration.CLIENT_SECRET_POST.equals(authMethod)) {
+                clientIdForRequest = registeredClientId;
+                clientSecretForRequest = registeredClientSecret;
+                useAuthorizationHeader = false;
+            }
+        } else if (oidcServerConfigurationUseCaseResult.isSuccess()
+                && oidcServerConfigurationUseCaseResult.getDataOrNull() != null
+                && oidcServerConfigurationUseCaseResult.getDataOrNull().isTokenEndpointAuthMethodSupportedClientSecretPost()) {
+            clientIdForRequest = clientId;
+            clientSecretForRequest = clientSecret;
+            useAuthorizationHeader = false;
+        }
+
         String clientAuth = OAuthUtils.Companion.getClientAuth(clientSecret, clientId);
 
         String scope;

owncloudApp/src/main/java/com/owncloud/android/presentation/authentication/AuthenticationViewModel.kt

@@ -26,11 +26,11 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.MediatorLiveData
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
-import com.owncloud.android.MainApp
 import com.owncloud.android.R
 import com.owncloud.android.domain.authentication.oauth.RegisterClientUseCase
 import com.owncloud.android.domain.authentication.oauth.RequestTokenUseCase
 import com.owncloud.android.domain.authentication.oauth.model.ClientRegistrationInfo
+import com.owncloud.android.domain.authentication.oauth.model.OIDCServerConfiguration
 import com.owncloud.android.domain.authentication.oauth.model.TokenRequest
 import com.owncloud.android.domain.authentication.oauth.model.TokenResponse
 import com.owncloud.android.domain.authentication.usecases.GetBaseUrlUseCase
@@ -39,6 +39,7 @@ import com.owncloud.android.domain.authentication.usecases.LoginOAuthAsyncUseCas
 import com.owncloud.android.domain.authentication.usecases.SupportsOAuth2UseCase
 import com.owncloud.android.domain.capabilities.usecases.GetStoredCapabilitiesUseCase
 import com.owncloud.android.domain.capabilities.usecases.RefreshCapabilitiesFromServerAsyncUseCase
+import com.owncloud.android.domain.exceptions.OAuth2ErrorException
 import com.owncloud.android.domain.server.model.ServerInfo
 import com.owncloud.android.domain.server.usecases.GetServerInfoAsyncUseCase
 import com.owncloud.android.domain.spaces.usecases.RefreshSpacesFromServerAsyncUseCase
@@ -231,12 +232,23 @@ class AuthenticationViewModel(
     )
 
     fun registerClient(
-        registrationEndpoint: String
+        registrationEndpoint: String,
+        oidcServerConfiguration: OIDCServerConfiguration,
     ) {
+        val tokenEndpointAuthMethod = try {
+            oidcServerConfiguration.selectRegistrationAuthMethod()
+        } catch (exception: OAuth2ErrorException) {
+            _registerClient.value = Event(UIResult.Error(error = exception))
+            return
+        }
+
+        // Replace any previous registration before dispatching so a failure cannot reuse its method.
+        _registerClient.value = Event(UIResult.Loading())
+
         val registrationRequest = OAuthUtils.buildClientRegistrationRequest(
             registrationEndpoint = registrationEndpoint,
-            MainApp.appContext
-        )
+            contextProvider.getContext()
+        ).copy(tokenEndpointAuthMethod = tokenEndpointAuthMethod)
 
         runUseCaseWithResult(
             coroutineDispatcher = coroutinesDispatcherProvider.io,

owncloudApp/src/main/java/com/owncloud/android/presentation/authentication/LoginActivity.kt

@@ -53,6 +53,7 @@ import com.owncloud.android.MainApp.Companion.accountType
 import com.owncloud.android.R
 import com.owncloud.android.data.authentication.KEY_USER_ID
 import com.owncloud.android.databinding.AccountSetupBinding
+import com.owncloud.android.domain.authentication.oauth.model.OIDCServerConfiguration
 import com.owncloud.android.domain.authentication.oauth.model.ResponseType
 import com.owncloud.android.domain.authentication.oauth.model.TokenRequest
 import com.owncloud.android.domain.authentication.oauth.model.TokenResponse
@@ -406,7 +407,8 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
                 if (registrationEndpoint != null) {
                     registerClient(
                         authorizationEndpoint = serverInfo.oidcServerConfiguration.authorizationEndpoint.toUri(),
-                        registrationEndpoint = registrationEndpoint
+                        registrationEndpoint = registrationEndpoint,
+                        oidcServerConfiguration = serverInfo.oidcServerConfiguration,
                     )
                 } else {
                     performGetAuthorizationCodeRequest(serverInfo.oidcServerConfiguration.authorizationEndpoint.toUri())
@@ -520,9 +522,13 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
      */
     private fun registerClient(
         authorizationEndpoint: Uri,
-        registrationEndpoint: String
+        registrationEndpoint: String,
+        oidcServerConfiguration: OIDCServerConfiguration,
     ) {
-        authenticationViewModel.registerClient(registrationEndpoint)
+        authenticationViewModel.registerClient(
+            registrationEndpoint = registrationEndpoint,
+            oidcServerConfiguration = oidcServerConfiguration,
+        )
         authenticationViewModel.registerClient.observe(this) {
             when (val uiResult = it.peekContent()) {
                 is UIResult.Loading -> {}
@@ -645,39 +651,53 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
     private fun exchangeAuthorizationCodeForTokens(authorizationCode: String) {
         binding.serverStatusText.text = getString(R.string.auth_getting_authorization)
 
-        val clientRegistrationInfo = authenticationViewModel.registerClient.value?.peekContent()?.getStoredData()
-
-        val clientAuth = if (clientRegistrationInfo?.clientId != null && clientRegistrationInfo.clientSecret != null) {
-            OAuthUtils.getClientAuth(clientRegistrationInfo.clientSecret as String, clientRegistrationInfo.clientId)
+        // Only a successful registration carries a method. Failures fall back to preconfigured credentials.
+        val clientRegistrationInfo =
+            (authenticationViewModel.registerClient.value?.peekContent() as? UIResult.Success)?.data
+        val registeredSecret = clientRegistrationInfo?.clientSecret
 
+        val clientAuth = if (clientRegistrationInfo != null && registeredSecret != null) {
+            OAuthUtils.getClientAuth(registeredSecret, clientRegistrationInfo.clientId)
+        } else if (isKiteworksServer) {
+            OAuthUtils.getClientAuth(getString(R.string.kiteworks_client_secret), getString(R.string.kiteworks_client_id))
         } else {
-            if (isKiteworksServer) {
-                OAuthUtils.getClientAuth(getString(R.string.kiteworks_client_secret), getString(R.string.kiteworks_client_id))
-            } else {
-                OAuthUtils.getClientAuth(getString(R.string.oauth2_client_secret), getString(R.string.oauth2_client_id))
-            }
+            OAuthUtils.getClientAuth(getString(R.string.oauth2_client_secret), getString(R.string.oauth2_client_id))
         }
 
         // Use oidc discovery one, or build an oauth endpoint using serverBaseUrl + Setup string.
-        val tokenEndPoint: String
+        // The registered auth method is resolved separately so later discovery cannot change it.
+        val serverInfo = authenticationViewModel.serverInfo.value?.peekContent()?.getStoredData()
+        val tokenEndPoint = if (serverInfo is ServerInfo.OIDCServer) {
+            serverInfo.oidcServerConfiguration.tokenEndpoint
+        } else {
+            "$serverBaseUrl${File.separator}${contextProvider.getString(R.string.oauth2_url_endpoint_access)}"
+        }
 
-        var clientId: String? = null
-        var clientSecret: String? = null
-        var useAuthorizationHeader = true
+        val useClientSecretPost = if (clientRegistrationInfo == null) {
+            serverInfo is ServerInfo.OIDCServer &&
+                serverInfo.oidcServerConfiguration.isTokenEndpointAuthMethodSupportedClientSecretPost()
+        } else {
+            clientRegistrationInfo.tokenEndpointAuthMethod == OIDCServerConfiguration.CLIENT_SECRET_POST
+        }
 
-        val serverInfo = authenticationViewModel.serverInfo.value?.peekContent()?.getStoredData()
-        if (serverInfo is ServerInfo.OIDCServer) {
-            tokenEndPoint = serverInfo.oidcServerConfiguration.tokenEndpoint
-            if (serverInfo.oidcServerConfiguration.isTokenEndpointAuthMethodSupportedClientSecretPost()) {
+        val clientId: String?
+        val clientSecret: String?
+        val useAuthorizationHeader: Boolean
+        if (useClientSecretPost) {
+            if (clientRegistrationInfo != null) {
+                clientId = clientRegistrationInfo.clientId
+                clientSecret = clientRegistrationInfo.clientSecret
+            } else {
                 val defaultClientId = if (isKiteworksServer) R.string.kiteworks_client_id else R.string.oauth2_client_id
                 val defaultClientSecret = if (isKiteworksServer) R.string.kiteworks_client_secret else R.string.oauth2_client_secret
-
-                clientId = clientRegistrationInfo?.clientId ?: contextProvider.getString(defaultClientId)
-                clientSecret = clientRegistrationInfo?.clientSecret ?: contextProvider.getString(defaultClientSecret)
-                useAuthorizationHeader = false
+                clientId = contextProvider.getString(defaultClientId)
+                clientSecret = contextProvider.getString(defaultClientSecret)
             }
+            useAuthorizationHeader = false
         } else {
-            tokenEndPoint = "$serverBaseUrl${File.separator}${contextProvider.getString(R.string.oauth2_url_endpoint_access)}"
+            clientId = null
+            clientSecret = null
+            useAuthorizationHeader = true
         }
 
         val requestToken = TokenRequest.AccessToken(

owncloudData/src/main/java/com/owncloud/android/data/authentication/AuthenticationConstants.kt

@@ -51,6 +51,7 @@ const val KEY_FEATURE_SPACES = "KEY_FEATURE_SPACES"
 const val KEY_CLIENT_REGISTRATION_CLIENT_ID = "client_id"
 const val KEY_CLIENT_REGISTRATION_CLIENT_SECRET = "client_secret"
 const val KEY_CLIENT_REGISTRATION_CLIENT_EXPIRATION_DATE = "client_secret_expires_at"
+const val KEY_CLIENT_REGISTRATION_TOKEN_ENDPOINT_AUTH_METHOD = "token_endpoint_auth_method"
 
 const val KEY_IS_KITEWORKS_SERVER = "is_kiteworks_server"
 

owncloudData/src/main/java/com/owncloud/android/data/authentication/datasources/implementation/OCLocalAuthenticationDataSource.kt

@@ -28,6 +28,7 @@ import android.net.Uri
 import com.owncloud.android.data.authentication.KEY_CLIENT_REGISTRATION_CLIENT_EXPIRATION_DATE
 import com.owncloud.android.data.authentication.KEY_CLIENT_REGISTRATION_CLIENT_ID
 import com.owncloud.android.data.authentication.KEY_CLIENT_REGISTRATION_CLIENT_SECRET
+import com.owncloud.android.data.authentication.KEY_CLIENT_REGISTRATION_TOKEN_ENDPOINT_AUTH_METHOD
 import com.owncloud.android.data.authentication.KEY_FEATURE_ALLOWED
 import com.owncloud.android.data.authentication.KEY_FEATURE_SPACES
 import com.owncloud.android.data.authentication.KEY_IS_KITEWORKS_SERVER
@@ -121,6 +122,11 @@ class OCLocalAuthenticationDataSource(
                     setUserData(it, KEY_CLIENT_REGISTRATION_CLIENT_ID, clientRegistrationInfo.clientId)
                     setUserData(it, KEY_CLIENT_REGISTRATION_CLIENT_SECRET, clientRegistrationInfo.clientSecret)
                     setUserData(it, KEY_CLIENT_REGISTRATION_CLIENT_EXPIRATION_DATE, clientRegistrationInfo.clientSecretExpiration.toString())
+                    setUserData(
+                        it,
+                        KEY_CLIENT_REGISTRATION_TOKEN_ENDPOINT_AUTH_METHOD,
+                        clientRegistrationInfo.tokenEndpointAuthMethod
+                    )
                 }
             }
 

owncloudData/src/main/java/com/owncloud/android/data/oauth/datasources/implementation/OCRemoteOAuthDataSource.kt

@@ -80,7 +80,7 @@ class OCRemoteOAuthDataSource(
             )
         }
 
-        return remoteClientRegistrationInfo.toModel()
+        return remoteClientRegistrationInfo.toModel(clientRegistrationRequest.tokenEndpointAuthMethod)
     }
 
     /**************************************************************************************************************
@@ -148,11 +148,12 @@ class OCRemoteOAuthDataSource(
             applicationType = this.applicationType
         )
 
-    private fun ClientRegistrationResponse.toModel(): ClientRegistrationInfo =
+    private fun ClientRegistrationResponse.toModel(tokenEndpointAuthMethod: String): ClientRegistrationInfo =
         ClientRegistrationInfo(
             clientId = this.clientId,
             clientSecret = this.clientSecret,
             clientIdIssuedAt = this.clientIdIssuedAt,
-            clientSecretExpiration = this.clientSecretExpiration
+            clientSecretExpiration = this.clientSecretExpiration,
+            tokenEndpointAuthMethod = tokenEndpointAuthMethod,
         )
 }

owncloudDomain/src/main/java/com/owncloud/android/domain/authentication/oauth/model/ClientRegistrationInfo.kt

@@ -24,4 +24,5 @@ data class ClientRegistrationInfo(
     val clientSecret: String?,
     val clientIdIssuedAt: Int?,
     val clientSecretExpiration: Int,
+    val tokenEndpointAuthMethod: String = OIDCServerConfiguration.CLIENT_SECRET_BASIC,
 )

owncloudDomain/src/main/java/com/owncloud/android/domain/authentication/oauth/model/OIDCServerConfiguration.kt

@@ -22,6 +22,8 @@
 
 package com.owncloud.android.domain.authentication.oauth.model
 
+import com.owncloud.android.domain.exceptions.OAuth2ErrorException
+
 data class OIDCServerConfiguration(
     val authorizationEndpoint: String,
     val checkSessionIframe: String?,
@@ -36,5 +38,26 @@ data class OIDCServerConfiguration(
     val isKiteworksServer: Boolean = false,
 ) {
     fun isTokenEndpointAuthMethodSupportedClientSecretPost(): Boolean =
-        tokenEndpointAuthMethodsSupported?.any { it == "client_secret_post" } ?: false
+        tokenEndpointAuthMethodsSupported?.any { it == CLIENT_SECRET_POST } ?: false
+
+    /**
+     * Auth method submitted with dynamic client registration.
+     * Basic wins when both supported methods are advertised. Omitted discovery metadata defaults to Basic.
+     * An explicit list that contains neither supported method cannot be registered.
+     */
+    fun selectRegistrationAuthMethod(): String {
+        val advertisedMethods = tokenEndpointAuthMethodsSupported ?: return CLIENT_SECRET_BASIC
+        if (advertisedMethods.any { it == CLIENT_SECRET_BASIC }) {
+            return CLIENT_SECRET_BASIC
+        }
+        if (advertisedMethods.any { it == CLIENT_SECRET_POST }) {
+            return CLIENT_SECRET_POST
+        }
+        throw OAuth2ErrorException()
+    }
+
+    companion object {
+        const val CLIENT_SECRET_BASIC = "client_secret_basic"
+        const val CLIENT_SECRET_POST = "client_secret_post"
+    }
 }