From 41d40ab5018f266a91132fe3383333dd6cd8d88e Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 Apr 2024 17:22:49 +0600 Subject: [PATCH 01/12] fix: OIDC authorizationService memory-leak issue OIDC authorizationService can leak memory if dump method is not called after end of usage. --- .../syncadapter/DefaultAccountAuthenticatorService.kt | 10 +++++++++- .../at/bitfire/davdroid/syncadapter/SyncManager.kt | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt index cc19d2dfa..a795b912a 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt @@ -32,6 +32,7 @@ import at.bitfire.davdroid.log.Logger import at.bitfire.davdroid.resource.LocalAddressBook import at.bitfire.davdroid.settings.AccountSettings import at.bitfire.davdroid.ui.setup.LoginActivity +import at.bitfire.davdroid.util.AuthStatePrefUtils import dagger.hilt.EntryPoint import dagger.hilt.InstallIn import dagger.hilt.android.EntryPointAccessors @@ -205,7 +206,9 @@ abstract class DefaultAccountAuthenticatorService : Service(), OnAccountsUpdateL val clientSecretString = accountManager.getUserData(account, AccountSettings.KEY_CLIENT_SECRET) val clientSecret = OpenIdUtils.getClientAuthentication(clientSecretString) - AuthorizationService(context).performTokenRequest( + val authorizationService = AuthorizationService(context) + + authorizationService.performTokenRequest( tokenRequest, clientSecret ) { tokenResponse, ex -> @@ -220,11 +223,16 @@ abstract class DefaultAccountAuthenticatorService : Service(), OnAccountsUpdateL AccountSettings.KEY_CLIENT_SECRET, clientSecretString ) + + AuthStatePrefUtils.saveAuthState(context, account, authState.jsonSerializeString()) + val result = Bundle() result.putString(AccountManager.KEY_ACCOUNT_NAME, account!!.name) result.putString(AccountManager.KEY_ACCOUNT_TYPE, account.type) result.putString(AccountManager.KEY_AUTHTOKEN, authState.accessToken) response?.onResult(result) + + authorizationService.dispose() } val result = Bundle() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncManager.kt index 32b18322f..503ae7fc4 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/SyncManager.kt @@ -194,7 +194,8 @@ abstract class SyncManager, out CollectionType: L val clientSecretString = accountSettings.credentials().clientSecret val clientSecret = OpenIdUtils.getClientAuthentication(clientSecretString) - AuthorizationService(context).performTokenRequest(tokenRequest, clientSecret) { tokenResponse, ex -> + val authorizationService = AuthorizationService(context) + authorizationService.performTokenRequest(tokenRequest, clientSecret) { tokenResponse, ex -> authState.update(tokenResponse, ex) accountSettings.credentials( Credentials( @@ -219,6 +220,8 @@ abstract class SyncManager, out CollectionType: L executor.execute { performSync(DEFAULT_RETRY_AFTER, DEFAULT_SECOND_RETRY_AFTER, DEFAULT_MAX_RETRY_TIME) } + + authorizationService.dispose() } } -- GitLab From d8e154e4f39d657e52f08fecb61bd5151135031b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 Apr 2024 17:36:16 +0600 Subject: [PATCH 02/12] feat: impl OIDC end session support On account logout for openID-connect (OIDC) account, the session is not cleared from the browser, so user might face unwanted behavior if s/he try to login/re-login to any account. To clear the session, we need to access the authState object, which is by default attached to osAccountManager's account, & removed after signOut. As we can only receive call back from onAccountRemovedBroadcast, authState value is removed. So we have to manually maintain a preference which holds the authState's value. (osAccountManager.account.authState is read by other system apps, but only this app update the values). clearSession opens the browser's intentSenderTab inside the app, which bind authorizationService. As broadCastManager can't bind services, we need to start OpenIdEndSessionActivity, which open the browserTab & bind the service. (P.S: this feature will only work for OIDC accounts which has endSessionURI declared by the provider; otherwise, it is ignored.) --- app/src/main/AndroidManifest.xml | 3 + .../receiver/AccountRemovedReceiver.kt | 29 +++++++++ .../davdroid/settings/AccountSettings.kt | 3 + .../ui/setup/AccountDetailsFragment.kt | 16 ++++- .../ui/signout/OpenIdEndSessionActivity.kt | 62 ++++++++++++++++++ .../davdroid/util/AuthStatePrefUtils.kt | 65 +++++++++++++++++++ 6 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 2039cf52d..69e39d803 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -14,6 +14,7 @@ + @@ -682,6 +683,8 @@ + + val authStateString = AuthStatePrefUtils.popAuthState(context, accountName, type) ?: return + startOIDCEndSessionActivity(context, authStateString) + } + } + + + private fun startOIDCEndSessionActivity(context: Context, authStateString: String) { + val intent = Intent(context, OpenIdEndSessionActivity::class.java) + intent.putExtra(AccountSettings.KEY_AUTH_STATE, authStateString) + intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + + context.applicationContext.startActivity(intent) } private fun getAccountName(context: Context, intent: Intent): String? { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt index a9ef5bb50..94d2571ea 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -19,6 +19,7 @@ import at.bitfire.davdroid.resource.LocalAddressBook import at.bitfire.davdroid.syncadapter.AccountUtils import at.bitfire.davdroid.syncadapter.PeriodicSyncWorker import at.bitfire.davdroid.syncadapter.SyncUtils +import at.bitfire.davdroid.util.AuthStatePrefUtils import at.bitfire.davdroid.util.SsoUtils import at.bitfire.davdroid.util.setAndVerifyUserData import at.bitfire.ical4android.TaskProvider @@ -271,6 +272,8 @@ class AccountSettings( // OAuth accountManager.setAndVerifyUserData(account, KEY_AUTH_STATE, credentials.authState?.jsonSerializeString()) accountManager.setAndVerifyUserData(account, KEY_CLIENT_SECRET, credentials.clientSecret) + + AuthStatePrefUtils.saveAuthState(context, account, credentials.authState?.jsonSerializeString()) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt index c6d17fb69..327163806 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt @@ -13,6 +13,7 @@ import android.content.ComponentName import android.content.ContentResolver import android.content.Context import android.content.Intent +import android.net.Uri import android.os.Bundle import android.provider.CalendarContract import android.text.Editable @@ -30,9 +31,11 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import androidx.lifecycle.viewModelScope +import at.bitfire.davdroid.BuildConfig import at.bitfire.davdroid.Constants import at.bitfire.davdroid.InvalidAccountException import at.bitfire.davdroid.R +import at.bitfire.davdroid.authorization.IdentityProvider import at.bitfire.davdroid.databinding.LoginAccountDetailsBinding import at.bitfire.davdroid.db.AppDatabase import at.bitfire.davdroid.db.Credentials @@ -47,6 +50,7 @@ import at.bitfire.davdroid.settings.SettingsManager import at.bitfire.davdroid.syncadapter.AccountUtils import at.bitfire.davdroid.syncadapter.SyncAllAccountWorker import at.bitfire.davdroid.syncadapter.SyncWorker +import at.bitfire.davdroid.util.AuthStatePrefUtils import at.bitfire.vcard4android.GroupMethod import com.google.android.material.snackbar.Snackbar import com.nextcloud.android.utils.AccountManagerUtils @@ -56,6 +60,8 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.launch +import net.openid.appauth.AuthorizationService +import net.openid.appauth.EndSessionRequest import java.util.logging.Level import javax.inject.Inject @@ -202,7 +208,6 @@ class AccountDetailsFragment : Fragment() { return v.root } - private fun notifyEdrive(name: String) { val intent = Intent("foundation.e.drive.action.ADD_ACCOUNT") intent.addFlags(Intent.FLAG_INCLUDE_STOPPED_PACKAGES) @@ -300,6 +305,9 @@ class AccountDetailsFragment : Fragment() { // create Android account val userData = AccountSettings.initialUserData(credentials, baseURL, config.cookies, config.calDAV?.emails?.firstOrNull()) + + AuthStatePrefUtils.saveAuthState(context, account, credentials?.authState?.jsonSerializeString()) + Logger.log.log(Level.INFO, "Creating Android account with initial config", arrayOf(account, userData)) val accountManager = AccountManager.get(context) @@ -424,14 +432,18 @@ class AccountDetailsFragment : Fragment() { if (userData.get(AccountSettings.KEY_EMAIL_ADDRESS) == accountManager .getUserData(account, AccountSettings.KEY_EMAIL_ADDRESS) ) { + val authState = userData.getString(AccountSettings.KEY_AUTH_STATE) + accountManager.setUserData( openIdAccount, AccountSettings.KEY_AUTH_STATE, - userData.getString(AccountSettings.KEY_AUTH_STATE) + authState ) accountManager.setUserData( openIdAccount, AccountSettings.KEY_CLIENT_SECRET, userData.getString(AccountSettings.KEY_CLIENT_SECRET) ) + + AuthStatePrefUtils.saveAuthState(context, openIdAccount, authState) } } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt new file mode 100644 index 000000000..3885c1447 --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt @@ -0,0 +1,62 @@ +/* + * Copyright MURENA SAS 2024 + * 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 at.bitfire.davdroid.ui.signout + +import android.app.Activity +import android.os.Bundle +import at.bitfire.davdroid.settings.AccountSettings +import net.openid.appauth.AuthState +import net.openid.appauth.AuthorizationService +import net.openid.appauth.EndSessionRequest + +class OpenIdEndSessionActivity : Activity() { + + private var authorizationService: AuthorizationService? = null + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + val authStateString = intent.getStringExtra(AccountSettings.KEY_AUTH_STATE) + + authStateString?.let { + val authState = AuthState.jsonDeserialize(it) + + val configuration = authState.authorizationServiceConfiguration + if (configuration?.endSessionEndpoint == null) { + finish() + return + } + + authorizationService = AuthorizationService(applicationContext) + + val intent = authorizationService!!.getEndSessionRequestIntent( + EndSessionRequest.Builder(configuration) + .setIdTokenHint(authState.idToken) + .build() + ) + + startActivity(intent) + } + + finish() + } + + override fun onDestroy() { + authorizationService?.dispose() + super.onDestroy() + } +} diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt new file mode 100644 index 000000000..e49b90df1 --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt @@ -0,0 +1,65 @@ +/* + * Copyright MURENA SAS 2024 + * 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 at.bitfire.davdroid.util + +import android.accounts.Account +import android.content.Context +import android.content.SharedPreferences + +object AuthStatePrefUtils { + + private const val AUTH_STATE_SHARED_PREF = "authStateShared_Pref" + + fun saveAuthState(context: Context, account: Account?, value: String?) { + if(account == null) { + return + } + + val preferences = getSharedPref(context) + preferences.edit() + .putString(getKey(account), value) + .apply() + } + + fun popAuthState(context: Context, name: String, type: String): String? { + val preferences = getSharedPref(context) + val key = getKey(name = name, type = type) + val value = preferences.getString(key, null) + val authState = if (value.isNullOrBlank()) null else value + + authState.let { + preferences.edit() + .remove(key) + .apply() + } + + return authState + } + + private fun getSharedPref(context: Context): SharedPreferences { + return context.getSharedPreferences(AUTH_STATE_SHARED_PREF, Context.MODE_PRIVATE) + } + + private fun getKey(account: Account): String { + return getKey(name = account.name, + type = account.type) + } + + private fun getKey(name: String, type: String): String { + return "$name==$type" + } +} -- GitLab From 81cdb51bac99be91b5f8554078bc2c4fd53eff7f Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 Apr 2024 17:38:15 +0600 Subject: [PATCH 03/12] chore: enable OIDC support for murenatest accounts Should not be merged on main, until prod is ready & murenatest related value should be replaced with murena serer value. --- app/build.gradle | 2 +- .../at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt | 2 +- .../at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index a2e14d695..3d1b5900b 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -219,7 +219,7 @@ dependencies { implementation "commons-httpclient:commons-httpclient:3.1@jar" // remove after entire switch to lib v2 implementation 'org.apache.jackrabbit:jackrabbit-webdav:2.13.5' // remove after entire switch to lib v2 implementation 'com.google.code.gson:gson:2.10.1' - implementation("foundation.e:Nextcloud-Android-Library:1.0.6-release") { + implementation("foundation.e:Nextcloud-Android-Library:1.0.7-alpha08") { exclude group: 'com.gitlab.bitfireAT', module: 'dav4jvm' exclude group: 'org.ogce', module: 'xpp3' // unused in Android and brings wrong Junit version exclude group: 'com.squareup.okhttp3' 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 c65296df1..53691500e 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 enableOpenIdSupport = false + const val enableOpenIdSupport = true } private var initialized = false diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt index 8a5636acc..91df861c1 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt @@ -49,6 +49,6 @@ class MurenaOpenIdAuthFragment : OpenIdAuthenticationBaseFragment(IdentityProvid return } - proceedNext(userName, "https://murena.io/remote.php/dav/files/$userName") + proceedNext(userName, "https://murenatest.io/remote.php/dav/files/$userName") } } -- GitLab From 5de7911b5024533065b99154a308fc9d870d1fb3 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 Apr 2024 18:12:57 +0600 Subject: [PATCH 04/12] chore: use library constant for cookie name & separator --- .../kotlin/at/bitfire/davdroid/settings/AccountSettings.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt index 94d2571ea..39dee0e6b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -119,8 +119,8 @@ class AccountSettings( const val CONTACTS_APP_INTERACTION = "z-app-generated--contactsinteraction--recent/" - const val COOKIE_KEY = "cookie_key" - const val COOKIE_SEPARATOR = "" + const val COOKIE_KEY = NCAccountUtils.Constants.KEY_OKHTTP_COOKIES + const val COOKIE_SEPARATOR = NCAccountUtils.Constants.OKHTTP_COOKIE_SEPARATOR /** Static property to indicate whether AccountSettings migration is currently running. * **Access must be `synchronized` with `AccountSettings::class.java`.** */ -- GitLab From f16e3f97626a17e54c1e54f654acb13a234d8a97 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 Apr 2024 22:10:41 +0600 Subject: [PATCH 05/12] chore: update according to review --- .../davdroid/receiver/AccountRemovedReceiver.kt | 8 ++------ .../syncadapter/DefaultAccountAuthenticatorService.kt | 4 ++-- .../at/bitfire/davdroid/util/AuthStatePrefUtils.kt | 10 +++------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt index 4a5bda170..0d30c3414 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt @@ -47,15 +47,11 @@ class AccountRemovedReceiver : BroadcastReceiver() { private fun handleOIDCSessionClearing( intent: Intent, - context: Context?, + context: Context, accountName: String ) { - if (context == null) { - return - } - intent.extras?.getString(AccountManager.KEY_ACCOUNT_TYPE)?.let { type -> - val authStateString = AuthStatePrefUtils.popAuthState(context, accountName, type) ?: return + val authStateString = AuthStatePrefUtils.loadAuthState(context, accountName, type) ?: return startOIDCEndSessionActivity(context, authStateString) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt index a795b912a..da635b072 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/syncadapter/DefaultAccountAuthenticatorService.kt @@ -224,10 +224,10 @@ abstract class DefaultAccountAuthenticatorService : Service(), OnAccountsUpdateL clientSecretString ) - AuthStatePrefUtils.saveAuthState(context, account, authState.jsonSerializeString()) + AuthStatePrefUtils.saveAuthState(context, account!!, authState.jsonSerializeString()) val result = Bundle() - result.putString(AccountManager.KEY_ACCOUNT_NAME, account!!.name) + result.putString(AccountManager.KEY_ACCOUNT_NAME, account.name) result.putString(AccountManager.KEY_ACCOUNT_TYPE, account.type) result.putString(AccountManager.KEY_AUTHTOKEN, authState.accessToken) response?.onResult(result) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt index e49b90df1..19ad813d7 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/AuthStatePrefUtils.kt @@ -24,21 +24,17 @@ object AuthStatePrefUtils { private const val AUTH_STATE_SHARED_PREF = "authStateShared_Pref" - fun saveAuthState(context: Context, account: Account?, value: String?) { - if(account == null) { - return - } - + fun saveAuthState(context: Context, account: Account, value: String?) { val preferences = getSharedPref(context) preferences.edit() .putString(getKey(account), value) .apply() } - fun popAuthState(context: Context, name: String, type: String): String? { + fun loadAuthState(context: Context, name: String, type: String): String? { val preferences = getSharedPref(context) val key = getKey(name = name, type = type) - val value = preferences.getString(key, null) + val value = preferences.getString(key, "") val authState = if (value.isNullOrBlank()) null else value authState.let { -- GitLab From 9dba74126a3ed6ecd585d57b1a3e0eefc5ce9d9c Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 3 Apr 2024 17:40:51 +0600 Subject: [PATCH 06/12] chore: refactor method name according to review --- .../at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt index 0d30c3414..e9a8d1aae 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt @@ -42,10 +42,10 @@ class AccountRemovedReceiver : BroadcastReceiver() { val ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton() ownCloudClientManager.removeClientForByName(accountName) - handleOIDCSessionClearing(intent, context, accountName) + clearOIDCSession(intent, context, accountName) } - private fun handleOIDCSessionClearing( + private fun clearOIDCSession( intent: Intent, context: Context, accountName: String -- GitLab From 3cfd40b6d6fc9fc19ff1f25fb52469592d50082b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 5 Apr 2024 12:12:08 +0600 Subject: [PATCH 07/12] chore: Refactor IdentityProvider - Add new env variables for baseUrl, logoutRedirectUri & discoveryEndPoint for murena account. So it will be easier to switch env from the CI/CD pipeline, no code changes required. - Use named parameters in IdentityProvider enum value initialization. - Retrieve identityProvider by accountType --- .gitlab-ci.yml | 3 + app/build.gradle | 7 +- .../authorization/IdentityProvider.kt | 77 ++++++++++++------- .../ui/setup/MurenaOpenIdAuthFragment.kt | 2 +- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6c74faced..8af528d99 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -9,6 +9,9 @@ before_script: - echo MURENA_CLIENT_ID=$MURENA_CLIENT_ID >> local.properties - echo MURENA_CLIENT_SECRET=$MURENA_CLIENT_SECRET >> local.properties - echo MURENA_REDIRECT_URI=$MURENA_REDIRECT_URI >> local.properties + - echo MURENA_LOGOUT_REDIRECT_URI=$MURENA_LOGOUT_REDIRECT_URI >> local.properties + - echo MURENA_BASE_URL=$MURENA_BASE_URL >> local.properties + - echo MURENA_DISCOVERY_END_POINT=$MURENA_DISCOVERY_END_POINT >> local.properties - echo GOOGLE_CLIENT_ID=$GOOGLE_CLIENT_ID >> local.properties - echo GOOGLE_REDIRECT_URI=$GOOGLE_REDIRECT_URI >> local.properties - echo YAHOO_CLIENT_ID=$YAHOO_CLIENT_ID >> local.properties diff --git a/app/build.gradle b/app/build.gradle index 3d1b5900b..f6538ba34 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -111,6 +111,9 @@ android { buildConfigField "String", "MURENA_CLIENT_ID", "\"${retrieveKey("MURENA_CLIENT_ID")}\"" buildConfigField "String", "MURENA_CLIENT_SECRET", "\"${retrieveKey("MURENA_CLIENT_SECRET")}\"" buildConfigField "String", "MURENA_REDIRECT_URI", "\"${retrieveKey("MURENA_REDIRECT_URI")}\"" + buildConfigField "String", "MURENA_LOGOUT_REDIRECT_URI", "\"${retrieveKey("MURENA_LOGOUT_REDIRECT_URI")}\"" + buildConfigField "String", "MURENA_BASE_URL", "\"${retrieveKey("MURENA_BASE_URL")}\"" + buildConfigField "String", "MURENA_DISCOVERY_END_POINT", "\"${retrieveKey("MURENA_DISCOVERY_END_POINT")}\"" buildConfigField "String", "GOOGLE_CLIENT_ID", "\"${retrieveKey("GOOGLE_CLIENT_ID")}\"" buildConfigField "String", "GOOGLE_REDIRECT_URI", "\"${retrieveKey("GOOGLE_REDIRECT_URI")}\"" @@ -120,7 +123,9 @@ android { manifestPlaceholders = [ 'appAuthRedirectScheme': applicationId, "googleAuthRedirectScheme": retrieveKey("GOOGLE_REDIRECT_URI"), - "murenaAuthRedirectScheme": retrieveKey("MURENA_REDIRECT_URI") + "murenaAuthRedirectScheme": retrieveKey("MURENA_REDIRECT_URI"), + + "murenaAuthLogoutRedirectScheme":retrieveKey("MURENA_LOGOUT_REDIRECT_URI") ] } 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 07141598e..f7fce1c16 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/authorization/IdentityProvider.kt @@ -15,8 +15,10 @@ */ package at.bitfire.davdroid.authorization +import android.content.Context import android.net.Uri import at.bitfire.davdroid.BuildConfig +import at.bitfire.davdroid.R import net.openid.appauth.AuthorizationServiceConfiguration import net.openid.appauth.AuthorizationServiceConfiguration.RetrieveConfigurationCallback @@ -30,40 +32,59 @@ enum class IdentityProvider( clientId: String, clientSecret: String?, redirectUri: String, + logoutRedirectUri: String?, scope: String, - userInfoEndpoint: String? + userInfoEndpoint: String?, + baseUrl: String? ) { MURENA( - "https://accounts.eeo.one/auth/realms/eeo.one/.well-known/openid-configuration", - null, - null, - BuildConfig.MURENA_CLIENT_ID, - BuildConfig.MURENA_CLIENT_SECRET, - BuildConfig.MURENA_REDIRECT_URI + ":/redirect", - "openid address profile email phone roles offline_access web-origins microprofile-jwt", - null + discoveryEndpoint = BuildConfig.MURENA_DISCOVERY_END_POINT, + authEndpoint = null, + tokenEndpoint = null, + clientId = BuildConfig.MURENA_CLIENT_ID, + clientSecret = BuildConfig.MURENA_CLIENT_SECRET, + redirectUri = BuildConfig.MURENA_REDIRECT_URI + ":/redirect", + logoutRedirectUri = BuildConfig.MURENA_LOGOUT_REDIRECT_URI + ":/redirect", + scope = "openid address profile email phone roles offline_access web-origins microprofile-jwt", + userInfoEndpoint = null, + baseUrl = BuildConfig.MURENA_BASE_URL, ), GOOGLE( - "https://accounts.google.com/.well-known/openid-configuration", - null, - null, - BuildConfig.GOOGLE_CLIENT_ID, - null, - BuildConfig.GOOGLE_REDIRECT_URI + ":/oauth2redirect", - "openid profile email https://www.googleapis.com/auth/carddav https://www.googleapis.com/auth/calendar https://mail.google.com/", - null + discoveryEndpoint = "https://accounts.google.com/.well-known/openid-configuration", + authEndpoint = null, + tokenEndpoint = null, + clientId = BuildConfig.GOOGLE_CLIENT_ID, + clientSecret = null, + redirectUri = BuildConfig.GOOGLE_REDIRECT_URI + ":/oauth2redirect", + logoutRedirectUri = null, + scope = "openid profile email https://www.googleapis.com/auth/carddav https://www.googleapis.com/auth/calendar https://mail.google.com/", + userInfoEndpoint = null, + baseUrl = null ), YAHOO( - "https://api.login.yahoo.com/.well-known/openid-configuration", - null, - null, - BuildConfig.YAHOO_CLIENT_ID, - null, - BuildConfig.APPLICATION_ID + "://oauth2redirect", - "openid openid2 profile email mail-w sdct-w ycal-w", - null + discoveryEndpoint = "https://api.login.yahoo.com/.well-known/openid-configuration", + authEndpoint = null, + tokenEndpoint = null, + clientId = BuildConfig.YAHOO_CLIENT_ID, + clientSecret = null, + redirectUri = BuildConfig.APPLICATION_ID + "://oauth2redirect", + logoutRedirectUri = null, + scope = "openid openid2 profile email mail-w sdct-w ycal-w", + userInfoEndpoint = null, + baseUrl = null ); + companion object { + fun retrieveByAccountType(context: Context, accountType: String): IdentityProvider? { + return when(accountType) { + context.getString(R.string.eelo_account_type) -> MURENA + context.getString(R.string.google_account_type) -> GOOGLE + context.getString(R.string.yahoo_account_type) -> YAHOO + else -> null + } + } + } + private val mDiscoveryEndpoint: Uri? private val mAuthEndpoint: Uri? private val mTokenEndpoint: Uri? @@ -71,8 +92,10 @@ enum class IdentityProvider( val clientId: String val clientSecret: String? val redirectUri: Uri + val logoutRedirectUri: Uri? val scope: String val userInfoEndpoint: String? + val baseUrl: String? init { require( @@ -87,8 +110,10 @@ enum class IdentityProvider( this.clientSecret = clientSecret this.redirectUri = retrieveUri(redirectUri) ?: throw IllegalArgumentException("invalid redirect uri") + this.logoutRedirectUri = retrieveUri(logoutRedirectUri) this.scope = scope this.userInfoEndpoint = userInfoEndpoint + this.baseUrl = baseUrl } fun retrieveConfig(callback: RetrieveConfigurationCallback) { @@ -105,4 +130,4 @@ enum class IdentityProvider( null } else Uri.parse(value) } -} \ No newline at end of file +} diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt index 91df861c1..ed1bea7a0 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/MurenaOpenIdAuthFragment.kt @@ -49,6 +49,6 @@ class MurenaOpenIdAuthFragment : OpenIdAuthenticationBaseFragment(IdentityProvid return } - proceedNext(userName, "https://murenatest.io/remote.php/dav/files/$userName") + proceedNext(userName, "${IdentityProvider.MURENA.baseUrl}$userName") } } -- GitLab From 5d2f4ea1787a01c68b427c7d8beaa07a053c7e81 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 5 Apr 2024 12:17:51 +0600 Subject: [PATCH 08/12] feat: Auto redirect from browser after authEndSession When user tries to logged out, the browser intentTab will pop up & end its session. After session cleared, the browser instance will close automatically. --- app/src/main/AndroidManifest.xml | 11 +++- .../receiver/AccountRemovedReceiver.kt | 31 +++++++---- .../ui/signout/OpenIdEndSessionActivity.kt | 55 +++++++++++++------ 3 files changed, 69 insertions(+), 28 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 69e39d803..30e04639b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -684,7 +684,16 @@ + android:exported="true" > + + + + + + + + + - val authStateString = AuthStatePrefUtils.loadAuthState(context, accountName, type) ?: return - startOIDCEndSessionActivity(context, authStateString) + val authStateString = + AuthStatePrefUtils.loadAuthState(context, accountName, type) ?: return + startOIDCEndSessionActivity( + context = context, + authState = authStateString, + accountType = type + ) } } - - private fun startOIDCEndSessionActivity(context: Context, authStateString: String) { + private fun startOIDCEndSessionActivity( + context: Context, + authState: String, + accountType: String + ) { val intent = Intent(context, OpenIdEndSessionActivity::class.java) - intent.putExtra(AccountSettings.KEY_AUTH_STATE, authStateString) + intent.putExtra(AccountSettings.KEY_AUTH_STATE, authState) + intent.putExtra(AccountManager.KEY_ACCOUNT_TYPE, accountType) intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) context.applicationContext.startActivity(intent) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt index 3885c1447..631e4b8f7 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/signout/OpenIdEndSessionActivity.kt @@ -16,11 +16,14 @@ package at.bitfire.davdroid.ui.signout +import android.accounts.AccountManager import android.app.Activity import android.os.Bundle +import at.bitfire.davdroid.authorization.IdentityProvider import at.bitfire.davdroid.settings.AccountSettings import net.openid.appauth.AuthState import net.openid.appauth.AuthorizationService +import net.openid.appauth.AuthorizationServiceConfiguration import net.openid.appauth.EndSessionRequest class OpenIdEndSessionActivity : Activity() { @@ -31,30 +34,50 @@ class OpenIdEndSessionActivity : Activity() { super.onCreate(savedInstanceState) val authStateString = intent.getStringExtra(AccountSettings.KEY_AUTH_STATE) + val accountType = intent.getStringExtra(AccountManager.KEY_ACCOUNT_TYPE) - authStateString?.let { - val authState = AuthState.jsonDeserialize(it) - - val configuration = authState.authorizationServiceConfiguration - if (configuration?.endSessionEndpoint == null) { - finish() - return - } - - authorizationService = AuthorizationService(applicationContext) + if (authStateString.isNullOrBlank() || accountType.isNullOrBlank()) { + finish() + return + } - val intent = authorizationService!!.getEndSessionRequestIntent( - EndSessionRequest.Builder(configuration) - .setIdTokenHint(authState.idToken) - .build() - ) + val authState = AuthState.jsonDeserialize(authStateString) - startActivity(intent) + val configuration = authState.authorizationServiceConfiguration + if (configuration?.endSessionEndpoint == null) { + finish() + return } + startEndSessionWebIntent( + accountType = accountType, + configuration = configuration, + authState = authState + ) + finish() } + private fun startEndSessionWebIntent( + accountType: String, + configuration: AuthorizationServiceConfiguration, + authState: AuthState + ) { + authorizationService = AuthorizationService(applicationContext) + + val redirectUri = + IdentityProvider.retrieveByAccountType(this, accountType)?.logoutRedirectUri + + val intent = authorizationService!!.getEndSessionRequestIntent( + EndSessionRequest.Builder(configuration) + .setIdTokenHint(authState.idToken) + .setPostLogoutRedirectUri(redirectUri) + .build() + ) + + startActivity(intent) + } + override fun onDestroy() { authorizationService?.dispose() super.onDestroy() -- GitLab From d2fdb8e4fe8d533bb4704cdaaea8c37b5a07ee12 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 5 Apr 2024 12:20:54 +0600 Subject: [PATCH 09/12] chore: fix acronym issue on OIDC method names --- .../bitfire/davdroid/receiver/AccountRemovedReceiver.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt index ecb6361fa..12917100d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt @@ -38,14 +38,14 @@ class AccountRemovedReceiver : BroadcastReceiver() { val ownCloudClientManager = OwnCloudClientManagerFactory.getDefaultSingleton() ownCloudClientManager.removeClientForByName(accountName) - clearOIDCSession( + clearOidcSession( intent = intent, context = context, accountName = accountName ) } - private fun clearOIDCSession( + private fun clearOidcSession( intent: Intent, context: Context, accountName: String @@ -53,7 +53,7 @@ class AccountRemovedReceiver : BroadcastReceiver() { intent.extras?.getString(AccountManager.KEY_ACCOUNT_TYPE)?.let { type -> val authStateString = AuthStatePrefUtils.loadAuthState(context, accountName, type) ?: return - startOIDCEndSessionActivity( + startOidcEndSessionActivity( context = context, authState = authStateString, accountType = type @@ -61,7 +61,7 @@ class AccountRemovedReceiver : BroadcastReceiver() { } } - private fun startOIDCEndSessionActivity( + private fun startOidcEndSessionActivity( context: Context, authState: String, accountType: String -- GitLab From aa2cd20b49bcded3f1978472068be21725e4a461 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 5 Apr 2024 12:22:55 +0600 Subject: [PATCH 10/12] chore: bump nc-android-lib version to 1.0.7-u2.17-release --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index f6538ba34..5ea75c8c4 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -224,7 +224,7 @@ dependencies { implementation "commons-httpclient:commons-httpclient:3.1@jar" // remove after entire switch to lib v2 implementation 'org.apache.jackrabbit:jackrabbit-webdav:2.13.5' // remove after entire switch to lib v2 implementation 'com.google.code.gson:gson:2.10.1' - implementation("foundation.e:Nextcloud-Android-Library:1.0.7-alpha08") { + implementation("foundation.e:Nextcloud-Android-Library:1.0.7-u2.17-release") { exclude group: 'com.gitlab.bitfireAT', module: 'dav4jvm' exclude group: 'org.ogce', module: 'xpp3' // unused in Android and brings wrong Junit version exclude group: 'com.squareup.okhttp3' -- GitLab From 736efdd0106d0efbda77b9acd47549b7b51b65e8 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 5 Apr 2024 12:24:37 +0600 Subject: [PATCH 11/12] chore: disable OIDC authentication As prod backend is not ready yet, but task is (hopefully) done in the app side, it is better to merge the changes. So we should not face any unwanted conflicts on merging big MRs in future. --- .../at/bitfire/davdroid/ui/setup/EeloAuthenticatorModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 53691500e..c65296df1 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 enableOpenIdSupport = true + const val enableOpenIdSupport = false } private var initialized = false -- GitLab From 405d3b0c414a9453a21505b48b8d73704018908b Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 15 Apr 2024 09:27:42 +0600 Subject: [PATCH 12/12] chore: update according to review --- .../bitfire/davdroid/receiver/AccountRemovedReceiver.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt index 12917100d..7409c058d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/receiver/AccountRemovedReceiver.kt @@ -67,9 +67,11 @@ class AccountRemovedReceiver : BroadcastReceiver() { accountType: String ) { val intent = Intent(context, OpenIdEndSessionActivity::class.java) - intent.putExtra(AccountSettings.KEY_AUTH_STATE, authState) - intent.putExtra(AccountManager.KEY_ACCOUNT_TYPE, accountType) - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + intent.apply { + putExtra(AccountSettings.KEY_AUTH_STATE, authState) + putExtra(AccountManager.KEY_ACCOUNT_TYPE, accountType) + addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + } context.applicationContext.startActivity(intent) } -- GitLab