Skip to content

Fix login wizard hickups #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guruz wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix login wizard hickups #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73f0bee
GetBaseUrlRemoteOperation: Fix logging
guruz Sep 15, 2026
32abdf0
Login: Register the token observer once, not per attempt
guruz Sep 16, 2026
76ebcbd
Login: Don't reopen browser on device rotate
guruz Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 93 additions & 53 deletions opencloudApp/src/main/java/eu/opencloud/android/presentation/authentication/LoginActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import eu.opencloud.android.databinding.AccountSetupBinding
import eu.opencloud.android.domain.authentication.oauth.model.ClientRegistrationInfo
import eu.opencloud.android.domain.authentication.oauth.model.ResponseType
import eu.opencloud.android.domain.authentication.oauth.model.TokenRequest
import eu.opencloud.android.domain.authentication.oauth.model.TokenResponse
import eu.opencloud.android.domain.exceptions.ForbiddenException
import eu.opencloud.android.domain.exceptions.NoNetworkConnectionException
import eu.opencloud.android.domain.exceptions.OpencloudVersionNotSupportedException
Expand All @@ -72,6 +73,7 @@ import eu.opencloud.android.domain.exceptions.SpecificForbiddenException
import eu.opencloud.android.domain.exceptions.UnauthorizedException
import eu.opencloud.android.domain.exceptions.UnhandledHttpCodeException
import eu.opencloud.android.domain.server.model.ServerInfo
import eu.opencloud.android.domain.utils.Event
import eu.opencloud.android.extensions.checkPasscodeEnforced
import eu.opencloud.android.extensions.goToUrl
import eu.opencloud.android.extensions.manageOptionLockSelected
Expand Down Expand Up @@ -114,6 +116,7 @@ private const val KEY_OIDC_SUPPORTED = "KEY_OIDC_SUPPORTED"
private const val KEY_CODE_VERIFIER = "KEY_CODE_VERIFIER"
private const val KEY_CODE_CHALLENGE = "KEY_CODE_CHALLENGE"
private const val KEY_OIDC_STATE = "KEY_OIDC_STATE"
private const val KEY_AUTHORIZATION_REQUEST_LAUNCHED = "KEY_AUTHORIZATION_REQUEST_LAUNCHED"
private const val KEY_AUTH_SERVER_BASE_URL = "KEY_AUTH_SERVER_BASE_URL"
private const val KEY_AUTH_OIDC_SUPPORTED = "KEY_AUTH_OIDC_SUPPORTED"
private const val KEY_AUTH_LOGIN_ACTION = "KEY_AUTH_LOGIN_ACTION"
Expand Down Expand Up @@ -167,6 +170,13 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
private var resultBundle: Bundle? = null
private var pendingAuthorizationIntent: Intent? = null

/**
* True once the browser has been opened for the current attempt. Kept across recreation so a
* replayed serverInfo result cannot launch a second authorization request: the activity is not
* configChanges-proof and its ViewModel re-delivers the last result to every new instance.
*/
private var authorizationRequestLaunched = false

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -207,6 +217,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
savedInstanceState.getString(KEY_CODE_VERIFIER)?.let { authenticationViewModel.codeVerifier = it }
savedInstanceState.getString(KEY_CODE_CHALLENGE)?.let { authenticationViewModel.codeChallenge = it }
savedInstanceState.getString(KEY_OIDC_STATE)?.let { authenticationViewModel.oidcState = it }
authorizationRequestLaunched = savedInstanceState.getBoolean(KEY_AUTHORIZATION_REQUEST_LAUNCHED)
}

// edge-to-edge
Expand Down Expand Up @@ -272,9 +283,9 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted

initBrandableOptionsUI()

binding.thumbnail.setOnClickListener { checkOcServer() }
binding.thumbnail.setOnClickListener { userRequestedServerCheck() }

binding.embeddedCheckServerButton.setOnClickListener { checkOcServer() }
binding.embeddedCheckServerButton.setOnClickListener { userRequestedServerCheck() }

setupHostUrlEnterAction()

Expand Down Expand Up @@ -494,6 +505,12 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
}
}

// Registered once, and with EventObserver so the result is consumed: a re-delivered or
// doubly-observed token response would otherwise start a second parallel login.
authenticationViewModel.requestToken.observe(this, Event.EventObserver { uiResult ->
onRequestTokenResult(uiResult)
})

authenticationViewModel.baseUrl.observe(this) { event ->
when (val uiResult = event.peekContent()) {
is UIResult.Loading -> {}
Expand Down Expand Up @@ -547,14 +564,24 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
val hardwareEnterPressed =
event?.keyCode == KeyEvent.KEYCODE_ENTER && event.action == KeyEvent.ACTION_DOWN
if (imeActionTriggered || hardwareEnterPressed) {
checkOcServer()
userRequestedServerCheck()
true
} else {
false
}
}
}

/**
* A server check the user actually asked for, which starts a new attempt and so may open the
* browser again. Kept apart from [checkOcServer] because that one is also reached from replayed
* LiveData observers after the activity is recreated, where re-opening the browser is the bug.
*/
private fun userRequestedServerCheck() {
authorizationRequestLaunched = false
checkOcServer()
}

private fun checkOcServer() {
val uri = binding.hostUrlInput.text.toString().trim()
if (uri.isNotEmpty()) {
Expand Down Expand Up @@ -861,6 +888,10 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
clientId: String = getString(R.string.oauth2_client_id),
webFingerScopes: List<String>? = null,
) {
if (authorizationRequestLaunched) {
Timber.d("Authorization request already launched for this attempt, not opening the browser again")
return
}
Timber.d("A browser should be opened now to authenticate this user.")

val customTabsBuilder: CustomTabsIntent.Builder = CustomTabsIntent.Builder()
Expand Down Expand Up @@ -902,6 +933,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
this,
authorizationEndpointUri
)
authorizationRequestLaunched = true
} catch (e: ActivityNotFoundException) {
binding.serverStatusText.visibility = INVISIBLE
showMessageInSnackbar(message = this.getString(R.string.file_list_no_app_for_perform_action))
Expand All @@ -923,6 +955,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
}

private fun handleGetAuthorizationCodeResponse(intent: Intent) {
authorizationRequestLaunched = false
val authorizationCode = intent.data?.getQueryParameter("code")
val state = intent.data?.getQueryParameter("state")

Expand Down Expand Up @@ -1011,60 +1044,66 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
)

authenticationViewModel.requestToken(requestToken)
}

authenticationViewModel.requestToken.observe(this) {
when (val uiResult = it.peekContent()) {
is UIResult.Loading -> {}
is UIResult.Success -> {
Timber.d("Tokens received ${uiResult.data}, trying to login, creating account and adding it to account manager")
val tokenResponse = uiResult.data ?: return@observe

// Extract preferred_username from id_token for login_hint on re-login
preferredUsername = extractPreferredUsernameFromIdToken(tokenResponse.idToken)
Timber.d("Preferred username from id_token: $preferredUsername")

// When webfinger provides a client_id without dynamic registration,
// store it so AccountAuthenticator can use it for token refresh
val effectiveClientRegistrationInfo = clientRegistrationInfo
?: (serverInfo as? ServerInfo.OIDCServer)?.webFingerClientId?.let { wfClientId ->
ClientRegistrationInfo(
clientId = wfClientId,
clientSecret = null,
clientIdIssuedAt = null,
clientSecretExpiration = 0,
)
}
/**
* Handles the result of the token request. Registered once in [initLiveDataObservers].
*/
private fun onRequestTokenResult(uiResult: UIResult<TokenResponse>) {
val clientRegistrationInfo = authenticationViewModel.registerClient.value?.peekContent()?.getStoredData()
val serverInfo = authenticationViewModel.serverInfo.value?.peekContent()?.getStoredData()

// Scope priority: webfinger scopes > MDM/string-resource > token response
val webFingerScopes = if (serverInfo is ServerInfo.OIDCServer) {
serverInfo.webFingerScopes
} else {
null
}
val effectiveScope = if (!oidcSupported) {
tokenResponse.scope
} else if (webFingerScopes != null) {
webFingerScopes.joinToString(" ")
} else {
mdmProvider.getBrandingString(CONFIGURATION_OAUTH2_OPEN_ID_SCOPE, R.string.oauth2_openid_scope)
when (uiResult) {
is UIResult.Loading -> {}
is UIResult.Success -> {
Timber.d("Tokens received ${uiResult.data}, trying to login, creating account and adding it to account manager")
val tokenResponse = uiResult.data ?: return

// Extract preferred_username from id_token for login_hint on re-login
preferredUsername = extractPreferredUsernameFromIdToken(tokenResponse.idToken)
Timber.d("Preferred username from id_token: $preferredUsername")

// When webfinger provides a client_id without dynamic registration,
// store it so AccountAuthenticator can use it for token refresh
val effectiveClientRegistrationInfo = clientRegistrationInfo
?: (serverInfo as? ServerInfo.OIDCServer)?.webFingerClientId?.let { wfClientId ->
ClientRegistrationInfo(
clientId = wfClientId,
clientSecret = null,
clientIdIssuedAt = null,
clientSecretExpiration = 0,
)
}

authenticationViewModel.loginOAuth(
serverBaseUrl = serverBaseUrl,
username = tokenResponse.additionalParameters?.get(KEY_USER_ID).orEmpty(),
authTokenType = OAUTH_TOKEN_TYPE,
accessToken = tokenResponse.accessToken,
refreshToken = tokenResponse.refreshToken.orEmpty(),
scope = effectiveScope,
updateAccountWithUsername = if (loginAction != ACTION_CREATE) userAccount?.name else null,
clientRegistrationInfo = effectiveClientRegistrationInfo
)
// Scope priority: webfinger scopes > MDM/string-resource > token response
val webFingerScopes = if (serverInfo is ServerInfo.OIDCServer) {
serverInfo.webFingerScopes
} else {
null
}

is UIResult.Error -> {
Timber.e(uiResult.error, "OAuth request to exchange authorization code for tokens failed")
updateOAuthStatusIconAndText(uiResult.error)
val effectiveScope = if (!oidcSupported) {
tokenResponse.scope
} else if (webFingerScopes != null) {
webFingerScopes.joinToString(" ")
} else {
mdmProvider.getBrandingString(CONFIGURATION_OAUTH2_OPEN_ID_SCOPE, R.string.oauth2_openid_scope)
}

authenticationViewModel.loginOAuth(
serverBaseUrl = serverBaseUrl,
username = tokenResponse.additionalParameters?.get(KEY_USER_ID).orEmpty(),
authTokenType = OAUTH_TOKEN_TYPE,
accessToken = tokenResponse.accessToken,
refreshToken = tokenResponse.refreshToken.orEmpty(),
scope = effectiveScope,
updateAccountWithUsername = if (loginAction != ACTION_CREATE) userAccount?.name else null,
clientRegistrationInfo = effectiveClientRegistrationInfo
)
}

is UIResult.Error -> {
Timber.e(uiResult.error, "OAuth request to exchange authorization code for tokens failed")
updateOAuthStatusIconAndText(uiResult.error)
}
}
}
Expand Down Expand Up @@ -1117,7 +1156,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted

override fun onSavedCertificate() {
Timber.d("Server certificate is trusted")
checkOcServer()
userRequestedServerCheck()
}

override fun onCancelCertificate() {
Expand Down Expand Up @@ -1173,7 +1212,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
binding.hostUrlFrame.isVisible = showInput
binding.centeredRefreshButton.isVisible = !showInput
if (!showInput) {
binding.centeredRefreshButton.setOnClickListener { checkOcServer() }
binding.centeredRefreshButton.setOnClickListener { userRequestedServerCheck() }
}

val url = mdmProvider.getBrandingString(mdmKey = CONFIGURATION_SERVER_URL, stringKey = R.string.server_url)
Expand Down Expand Up @@ -1331,6 +1370,7 @@ class LoginActivity : AppCompatActivity(), SslUntrustedCertDialog.OnSslUntrusted
outState.putString(KEY_CODE_VERIFIER, authenticationViewModel.codeVerifier)
outState.putString(KEY_CODE_CHALLENGE, authenticationViewModel.codeChallenge)
outState.putString(KEY_OIDC_STATE, authenticationViewModel.oidcState)
outState.putBoolean(KEY_AUTHORIZATION_REQUEST_LAUNCHED, authorizationRequestLaunched)
outState.putString(KEY_AUTH_MTLS_CERT_ALIAS, clientCertAlias)
outState.putBoolean(KEY_AUTH_MTLS_CERT_ALIAS_CHANGED, clientCertAliasChangedByUser)
}
Expand Down
3 changes: 0 additions & 3 deletions ...brary/src/main/java/eu/opencloud/android/lib/common/operations/RemoteOperationResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,6 @@ private RemoteOperationResult(int httpCode, String httpPhrase) {
case HttpConstants.HTTP_LOCKED: // 423
mCode = ResultCode.RESOURCE_LOCKED;
break;
case HttpConstants.HTTP_INTERNAL_SERVER_ERROR: // 500
mCode = ResultCode.UNHANDLED_HTTP_CODE; // treat as generic server error
break;
case HttpConstants.HTTP_SERVICE_UNAVAILABLE: // 503
mCode = ResultCode.SERVICE_UNAVAILABLE;
break;
Expand Down
1 change: 1 addition & 0 deletions ...brary/src/main/java/eu/opencloud/android/lib/resources/files/GetBaseUrlRemoteOperation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class GetBaseUrlRemoteOperation : RemoteOperation<String?>() {
data = propFindMethod.getFinalUrl().toString()
}
} else {
Timber.e("Could not get base URL from $stringUrl, PROPFIND finished with HTTP status $status")
RemoteOperationResult<String?>(propFindMethod).apply {
data = null
}
Expand Down
Loading