From a9f529724ed43a3a5da3270fe4b20bdb73a1a2f3 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 3 Apr 2024 20:28:16 +0600 Subject: [PATCH 1/5] feat: retrive userId from caldav/carddav pricipal url instead of sanitizing userName Typically nextcloud userId should be first part of it's email address. It can be modified by the backend. So if the userId is changed, the eDrive & Notes app will stop working properly. On the other-hand, CalDav & CardDav principal url should contains the userID in the end. So, we can retrieve the userId from there. - Improve SssoGrantPermissionActivity to make it easier to maintain. issue: https://gitlab.e.foundation/e/os/backlog/-/issues/1975 --- .../activity/SsoGrantPermissionActivity.java | 174 ------------------ .../ui/activity/SsoGrantPermissionActivity.kt | 80 ++++++++ .../ui/activity/SsoGrantPermissionEvent.kt | 26 +++ .../activity/SsoGrantPermissionViewModel.kt | 174 ++++++++++++++++++ .../davdroid/settings/AccountSettings.kt | 11 +- .../ui/setup/AccountDetailsFragment.kt | 11 +- .../at/bitfire/davdroid/util/SsoUtils.kt | 48 ----- .../at/bitfire/davdroid/util/UserIdFetcher.kt | 50 +++++ .../at/bitfire/davdroid/util/SsoUtilsTest.kt | 58 ------ .../davdroid/util/UserIdFetcherTest.kt | 62 +++++++ 10 files changed, 404 insertions(+), 290 deletions(-) delete mode 100644 app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.java create mode 100644 app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt create mode 100644 app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionEvent.kt create mode 100644 app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt delete mode 100644 app/src/main/kotlin/at/bitfire/davdroid/util/SsoUtils.kt create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt delete mode 100644 app/src/test/kotlin/at/bitfire/davdroid/util/SsoUtilsTest.kt create mode 100644 app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.java deleted file mode 100644 index 8c1de631f..000000000 --- a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.java +++ /dev/null @@ -1,174 +0,0 @@ -/* - * Copyright MURENA SAS 2023 - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.owncloud.android.ui.activity; - -import static com.nextcloud.android.sso.Constants.DELIMITER; -import static com.nextcloud.android.sso.Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED; -import static com.nextcloud.android.sso.Constants.EXCEPTION_ACCOUNT_NOT_FOUND; -import static com.nextcloud.android.sso.Constants.NEXTCLOUD_FILES_ACCOUNT; -import static com.nextcloud.android.sso.Constants.NEXTCLOUD_SSO; -import static com.nextcloud.android.sso.Constants.NEXTCLOUD_SSO_EXCEPTION; -import static com.nextcloud.android.sso.Constants.SSO_SERVER_URL; -import static com.nextcloud.android.sso.Constants.SSO_SHARED_PREFERENCE; -import static com.nextcloud.android.sso.Constants.SSO_TOKEN; -import static com.nextcloud.android.sso.Constants.SSO_USER_ID; - -import android.accounts.Account; -import android.accounts.AccountManager; -import android.content.ComponentName; -import android.content.Context; -import android.content.Intent; -import android.content.SharedPreferences; -import android.os.Bundle; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.appcompat.app.AppCompatActivity; - -import com.nextcloud.android.utils.EncryptionUtils; -import com.owncloud.android.lib.common.OwnCloudAccount; -import com.owncloud.android.lib.common.accounts.AccountUtils; - -import java.util.Arrays; -import java.util.UUID; -import java.util.logging.Level; - -import at.bitfire.davdroid.R; -import at.bitfire.davdroid.log.Logger; -import at.bitfire.davdroid.util.SsoUtils; - -public class SsoGrantPermissionActivity extends AppCompatActivity { - - private static final String[] ACCEPTED_PACKAGE_LIST = {"foundation.e.notes"}; - - private Account account; - private String packageName; - - @Override - protected void onCreate(@Nullable Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - setContentView(R.layout.activity_sso_grant_permission); - - ComponentName callingActivity = getCallingActivity(); - - if (callingActivity != null) { - packageName = callingActivity.getPackageName(); - account = getIntent().getParcelableExtra(NEXTCLOUD_FILES_ACCOUNT); - validateAndAutoGrandPermission(); - } else { - Logger.INSTANCE.getLog().log(Level.SEVERE, "SsoGrantPermissionActivity: Calling Package is null"); - setResultAndExit(EXCEPTION_ACCOUNT_ACCESS_DECLINED); - } - } - - private void validateAndAutoGrandPermission() { - if (!isValidRequest()) { - Logger.INSTANCE.getLog().log(Level.SEVERE, "SsoGrantPermissionActivity: Invalid request"); - setResultAndExit(EXCEPTION_ACCOUNT_ACCESS_DECLINED); - return; - } - - grantPermission(); - } - - private boolean isValidRequest() { - if (packageName == null || account == null) { - return false; - } - - boolean validPackage = Arrays.asList(ACCEPTED_PACKAGE_LIST) - .contains(packageName); - - if (!validPackage) { - return false; - } - - return Arrays.asList(getAcceptedAccountTypeList()) - .contains(account.type); - } - - private String[] getAcceptedAccountTypeList() { - return new String[]{ - getString(R.string.eelo_account_type) - }; - } - - private void grantPermission() { - String serverUrl = getServerUrl(); - - if (serverUrl == null) { - return; - } - - // create token - String token = UUID.randomUUID().toString().replaceAll("-", ""); - String userId = getUserId(); - - saveToken(token, account.name); - setResultData(token, userId, serverUrl); - finish(); - } - - @NonNull - private String getUserId() { - final AccountManager accountManager = AccountManager.get(this); - final String baseUrl = accountManager.getUserData(account, AccountUtils.Constants.KEY_OC_BASE_URL); - return SsoUtils.INSTANCE.sanitizeUserId(account.name, baseUrl); - } - - private void setResultData(String token, String userId, String serverUrl) { - final Bundle result = new Bundle(); - result.putString(AccountManager.KEY_ACCOUNT_NAME, account.name); - result.putString(AccountManager.KEY_ACCOUNT_TYPE, account.type); - result.putString(AccountManager.KEY_AUTHTOKEN, NEXTCLOUD_SSO); - result.putString(SSO_USER_ID, userId); - result.putString(SSO_TOKEN, token); - result.putString(SSO_SERVER_URL, serverUrl); - - Intent data = new Intent(); - data.putExtra(NEXTCLOUD_SSO, result); - setResult(RESULT_OK, data); - } - - @Nullable - private String getServerUrl() { - try { - OwnCloudAccount ocAccount = new OwnCloudAccount(account, this); - return ocAccount.getBaseUri().toString(); - } catch (AccountUtils.AccountNotFoundException e) { - Logger.INSTANCE.getLog().log(Level.SEVERE, "SsoGrantPermissionActivity: Account not found"); - setResultAndExit(EXCEPTION_ACCOUNT_NOT_FOUND); - } - - return null; - } - - private void saveToken(String token, String accountName) { - String hashedTokenWithSalt = EncryptionUtils.generateSHA512(token); - SharedPreferences sharedPreferences = getSharedPreferences(SSO_SHARED_PREFERENCE, Context.MODE_PRIVATE); - SharedPreferences.Editor editor = sharedPreferences.edit(); - editor.putString(packageName + DELIMITER + accountName, hashedTokenWithSalt); - editor.apply(); - } - - private void setResultAndExit(String exception) { - Intent data = new Intent(); - data.putExtra(NEXTCLOUD_SSO_EXCEPTION, exception); - setResult(RESULT_CANCELED, data); - finish(); - } -} \ No newline at end of file diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt new file mode 100644 index 000000000..485a173ed --- /dev/null +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt @@ -0,0 +1,80 @@ +/* + * 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 com.owncloud.android.ui.activity + +import android.accounts.Account +import android.content.Intent +import android.os.Build +import android.os.Bundle +import androidx.activity.viewModels +import androidx.appcompat.app.AppCompatActivity +import androidx.lifecycle.lifecycleScope +import at.bitfire.davdroid.R +import com.nextcloud.android.sso.Constants +import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.launch + +@AndroidEntryPoint +class SsoGrantPermissionActivity: AppCompatActivity() { + + private val viewModel: SsoGrantPermissionViewModel by viewModels() + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setContentView(R.layout.activity_sso_grant_permission) + + lifecycleScope.launch { + viewModel.event.collectLatest { + when (it) { + is SsoGrantPermissionEvent.PermissionGranted -> setSuccessResult(it.bundle) + is SsoGrantPermissionEvent.PermissionDenied -> setCanceledResult(it.errorMessage) + } + } + } + + initiateValidation() + } + + private fun initiateValidation() { + val account: Account? = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + intent.getParcelableExtra(Constants.NEXTCLOUD_FILES_ACCOUNT, Account::class.java) + } else { + intent.getParcelableExtra(Constants.NEXTCLOUD_FILES_ACCOUNT) + } + + viewModel.initValidation( + callingActivity = callingActivity, + account = account + ) + } + + private fun setCanceledResult(exception: String) { + val data = Intent() + data.putExtra(Constants.NEXTCLOUD_SSO_EXCEPTION, exception) + setResult(RESULT_CANCELED, data) + finish() + } + + private fun setSuccessResult(result: Bundle) { + val data = Intent() + data.putExtra(Constants.NEXTCLOUD_SSO, result) + setResult(RESULT_OK, data) + finish() + } +} diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionEvent.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionEvent.kt new file mode 100644 index 000000000..2e69754c8 --- /dev/null +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionEvent.kt @@ -0,0 +1,26 @@ +/* + * 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 com.owncloud.android.ui.activity + +import android.os.Bundle + +sealed class SsoGrantPermissionEvent { + + data class PermissionGranted(val bundle: Bundle) : SsoGrantPermissionEvent() + + data class PermissionDenied(val errorMessage: String) : SsoGrantPermissionEvent() +} diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt new file mode 100644 index 000000000..94f124a8b --- /dev/null +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt @@ -0,0 +1,174 @@ +/* + * 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 com.owncloud.android.ui.activity + +import android.accounts.Account +import android.accounts.AccountManager +import android.content.ComponentName +import android.content.Context +import android.os.Bundle +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import at.bitfire.davdroid.R +import at.bitfire.davdroid.db.AppDatabase +import at.bitfire.davdroid.log.Logger.log +import at.bitfire.davdroid.util.UserIdFetcher +import com.nextcloud.android.sso.Constants +import com.nextcloud.android.utils.EncryptionUtils +import com.owncloud.android.lib.common.OwnCloudAccount +import com.owncloud.android.lib.common.accounts.AccountUtils +import dagger.hilt.android.lifecycle.HiltViewModel +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.launch +import java.util.UUID +import java.util.logging.Level +import javax.inject.Inject + +@HiltViewModel +class SsoGrantPermissionViewModel @Inject constructor( + @ApplicationContext private val context: Context, + private val database: AppDatabase, +): ViewModel() { + + private val acceptedAccountTypes = listOf(context.getString(R.string.eelo_account_type)) + private val acceptedPackages = listOf("foundation.e.notes") + + private val _event = MutableSharedFlow() + val event = _event.asSharedFlow() + + fun initValidation(callingActivity: ComponentName?, account: Account?) { + viewModelScope.launch(Dispatchers.IO) { + val packageName = getCallingPackageName(callingActivity) ?: return@launch + validate(packageName, account) + } + } + + private suspend fun getCallingPackageName(callingActivity: ComponentName?): String? { + if (callingActivity == null) { + log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Calling Package is null") + _event.emit( + SsoGrantPermissionEvent.PermissionDenied( + errorMessage = Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED + ) + ) + return null + } + + return callingActivity.packageName + } + + private suspend fun validate(packageName: String?, account: Account?) { + if (!isValidRequest(packageName, account)) { + log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Invalid request") + _event.emit( + SsoGrantPermissionEvent.PermissionDenied( + errorMessage = Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED + ) + ) + } + + val serverUrl = getServerUrl(account!!) ?: return + + val token = UUID.randomUUID().toString().replace("-".toRegex(), "") + val userId = getUserId(account) + + saveToken( + token = token, + accountName = userId, + packageName = packageName!! + ) + + passSuccessfulData( + account = account, + token = token, + userId = userId, + serverUrl = serverUrl + ) + + } + + private fun isValidRequest(packageName: String?, account: Account?): Boolean { + if (packageName == null || account == null) { + return false + } + + return acceptedPackages.contains(packageName) && acceptedAccountTypes.contains(account.type) + } + + private suspend fun getServerUrl(account: Account): String? { + try { + val ocAccount = OwnCloudAccount(account, context) + return ocAccount.baseUri.toString() + } catch (e: AccountUtils.AccountNotFoundException) { + log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Account not found") + _event.emit( + SsoGrantPermissionEvent.PermissionDenied( + errorMessage = Constants.EXCEPTION_ACCOUNT_NOT_FOUND + ) + ) + } + return null + } + + private fun getUserId(account: Account): String { + val accountManager = AccountManager.get(context) + val userId = accountManager.getUserData(account, AccountUtils.Constants.KEY_USER_ID) + + if (!userId.isNullOrBlank()) { + return userId + } + + val principalUrl = + database.serviceDao().getByAccountName(account.name)?.principal?.toString() + ?: return account.name + + return UserIdFetcher.retrieveUserId(principalUrl) ?: account.name + } + + private fun saveToken(token: String, accountName: String, packageName: String) { + val hashedTokenWithSalt = EncryptionUtils.generateSHA512(token) + val sharedPreferences = + context.getSharedPreferences(Constants.SSO_SHARED_PREFERENCE, Context.MODE_PRIVATE) + val editor = sharedPreferences.edit() + editor.putString(packageName + Constants.DELIMITER + accountName, hashedTokenWithSalt) + editor.apply() + } + + private suspend fun passSuccessfulData( + account: Account, + token: String, + userId: String, + serverUrl: String + ) { + val result = Bundle() + result.putString(AccountManager.KEY_ACCOUNT_NAME, account.name) + result.putString(AccountManager.KEY_ACCOUNT_TYPE, account.type) + result.putString(AccountManager.KEY_AUTHTOKEN, Constants.NEXTCLOUD_SSO) + result.putString(Constants.SSO_USER_ID, userId) + result.putString(Constants.SSO_TOKEN, token) + result.putString(Constants.SSO_SERVER_URL, serverUrl) + + _event.emit( + SsoGrantPermissionEvent.PermissionGranted( + bundle = result + ) + ) + } +} 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..613643aba 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -19,7 +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.SsoUtils +import at.bitfire.davdroid.util.UserIdFetcher import at.bitfire.davdroid.util.setAndVerifyUserData import at.bitfire.ical4android.TaskProvider import at.bitfire.vcard4android.GroupMethod @@ -163,15 +163,16 @@ class AccountSettings( bundle.putString(NCAccountUtils.Constants.KEY_OC_BASE_URL, baseUrl) } - addUserIdToBundle(bundle, userName, baseUrl) + addUserIdToBundle(bundle, userName, url) addEmailToBundle(bundle, email, userName) return bundle } - private fun addUserIdToBundle(bundle: Bundle, userName: String?, baseUrl: String?) { - userName?.let { - val userId = SsoUtils.sanitizeUserId(it, baseUrl) + private fun addUserIdToBundle(bundle: Bundle, userName: String?, url: String?) { + val userId = UserIdFetcher.retrieveUserId(url) ?: userName + + userId?.let { bundle.putString(NCAccountUtils.Constants.KEY_USER_ID, userId) } } 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..dfcbb66a7 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 @@ -275,14 +275,17 @@ class AccountDetailsFragment : Fragment() { var baseURL : String? = null if (config.calDAV != null) { - baseURL = config.calDAV.principal.toString() + baseURL = config.calDAV.principal?.toString() + } + + if (baseURL == null) { + baseURL = config.cardDAV?.principal?.toString() } when (activity.intent.getStringExtra(LoginActivity.ACCOUNT_TYPE)) { context.getString(R.string.eelo_account_type) -> { accountType = context.getString(R.string.eelo_account_type) addressBookAccountType = context.getString(R.string.account_type_eelo_address_book) - baseURL = credentials?.serverUri.toString() } context.getString(R.string.google_account_type) -> { accountType = context.getString(R.string.google_account_type) @@ -462,7 +465,5 @@ class AccountDetailsFragment : Fragment() { return serviceId } - } - -} \ No newline at end of file +} diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/SsoUtils.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/SsoUtils.kt deleted file mode 100644 index 7836933cd..000000000 --- a/app/src/main/kotlin/at/bitfire/davdroid/util/SsoUtils.kt +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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 java.net.URI - -object SsoUtils { - - /** - * This method removes the baseUrl's host part from the accountName to retrieve userId - * for ex: accountName: abc@murena.io; baseUrl: https://murena.io; so userID: abc - */ - fun sanitizeUserId(accountName: String, baseUrl: String?): String { - val userId = accountName.trim() - - val host = getHost(baseUrl) ?: return userId - val userNameEndPart = "@$host" - - if (!userId.endsWith(userNameEndPart, ignoreCase = true)) { - return userId - } - - return userId.substring(0, userId.lastIndexOf(userNameEndPart, ignoreCase = true)) - } - - private fun getHost(baseUrl: String?): String? { - if (baseUrl == null) { - return null - } - - val uri = URI.create(baseUrl.trim()) - return uri.host - } -} diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt new file mode 100644 index 000000000..184ded411 --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt @@ -0,0 +1,50 @@ +/* + * 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 + +object UserIdFetcher { + + /** + * retrieve the userId from caldav/carddav nextcloud principal url. + * example: if the url is: https://abc.com/remote.php/dav/principals/users/xyz/, then this function will return xyz. + * + * this function will return null, if + * 1. provided url is null + * 2. `/users/` part is missing + */ + fun retrieveUserId(principalUrl: String?): String? { + if (principalUrl == null) { + return null + } + + val usersPart = "/users/" + + var userId: String? = null + if (principalUrl.contains(usersPart, ignoreCase = true)) { + userId = principalUrl.split(usersPart, ignoreCase = true)[1] + if (userId.endsWith("/")) { + userId = userId.dropLast(1) + } + + if (userId.isBlank()) { + userId = null + } + } + + return userId + } +} diff --git a/app/src/test/kotlin/at/bitfire/davdroid/util/SsoUtilsTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/util/SsoUtilsTest.kt deleted file mode 100644 index e14b20e9d..000000000 --- a/app/src/test/kotlin/at/bitfire/davdroid/util/SsoUtilsTest.kt +++ /dev/null @@ -1,58 +0,0 @@ -/* - * 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 org.junit.Assert.assertEquals -import org.junit.Test - -class SsoUtilsTest { - - @Test - fun `test sanitizeUserId with empty input`() { - val userId = "" - val expected = "" - val actual = SsoUtils.sanitizeUserId(userId, null) - assertEquals(expected, actual) - } - - @Test - fun `test sanitizeUserId with input without url part`() { - val userId = "username" - val url = "https://murena.io" - val expected = "username" - val actual = SsoUtils.sanitizeUserId(userId, url) - assertEquals(expected, actual) - } - - @Test - fun `test sanitizeUserId with case sensitivity`() { - val userId = "User@E.EMAIL@MURENA.io" - val baseUrl = "https://murena.io/" - val expected = "User@E.EMAIL" - val actual = SsoUtils.sanitizeUserId(userId, baseUrl) - assertEquals(expected, actual) - } - - @Test - fun `test sanitizeUserId with leading or trailing spaces`() { - val userId = " user@domain.com " - val url = " http://domain.com " - val expected = "user" - val actual = SsoUtils.sanitizeUserId(userId, url) - assertEquals(expected, actual) - } -} diff --git a/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt new file mode 100644 index 000000000..1c809efe0 --- /dev/null +++ b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.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.util + +import org.junit.Assert.assertEquals +import org.junit.Test + +class UserIdFetcherTest { + + @Test + fun `test retrieveUserId with empty input`() { + val expected = null + val actual = UserIdFetcher.retrieveUserId(null) + assertEquals(expected, actual) + } + + @Test + fun `test retrieveUserId with input without 'users' part`() { + val url = "https://murena.io" + val expected = null + val actual = UserIdFetcher.retrieveUserId(url) + assertEquals(expected, actual) + } + + @Test + fun `test retrieveUserId with input ends with 'users'`() { + val baseUrl = "https://murena.io/remote.php/dav/principals/users/" + val expected = null + val actual = UserIdFetcher.retrieveUserId(baseUrl) + assertEquals(expected, actual) + } + + @Test + fun `test retrieveUserId with input with 'users' part`() { + val baseUrl = "https://murena.io/remote.php/dav/principals/users/user@e.email" + val expected = "user@e.email" + val actual = UserIdFetcher.retrieveUserId(baseUrl) + assertEquals(expected, actual) + } + + @Test + fun `test retrieveUserId with input with 'users' part & ends with slash`() { + val baseUrl = "https://murena.io/remote.php/dav/principals/users/user@e.email/" + val expected = "user@e.email" + val actual = UserIdFetcher.retrieveUserId(baseUrl) + assertEquals(expected, actual) + } +} -- GitLab From 5ca97328fb7ea66dcdd80e0c646ae2b393f62403 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 8 Apr 2024 11:55:40 +0600 Subject: [PATCH 2/5] fix: flow leak on SsoGrantActivity by binding with lifeCycle --- .../ui/activity/SsoGrantPermissionActivity.kt | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt index 485a173ed..05e63af61 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt @@ -22,6 +22,8 @@ import android.os.Build import android.os.Bundle import androidx.activity.viewModels import androidx.appcompat.app.AppCompatActivity +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import at.bitfire.davdroid.R import com.nextcloud.android.sso.Constants @@ -30,7 +32,7 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch @AndroidEntryPoint -class SsoGrantPermissionActivity: AppCompatActivity() { +class SsoGrantPermissionActivity : AppCompatActivity() { private val viewModel: SsoGrantPermissionViewModel by viewModels() @@ -39,12 +41,16 @@ class SsoGrantPermissionActivity: AppCompatActivity() { setContentView(R.layout.activity_sso_grant_permission) lifecycleScope.launch { - viewModel.event.collectLatest { - when (it) { - is SsoGrantPermissionEvent.PermissionGranted -> setSuccessResult(it.bundle) - is SsoGrantPermissionEvent.PermissionDenied -> setCanceledResult(it.errorMessage) + viewModel.event + .flowWithLifecycle( + lifecycle = lifecycle, + minActiveState = Lifecycle.State.CREATED + ).collectLatest { + when (it) { + is SsoGrantPermissionEvent.PermissionGranted -> setSuccessResult(it.bundle) + is SsoGrantPermissionEvent.PermissionDenied -> setCanceledResult(it.errorMessage) + } } - } } initiateValidation() -- GitLab From 1c849ae8d52b5481760a2a44f3225eed2f75f13e Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 9 Apr 2024 09:17:22 +0600 Subject: [PATCH 3/5] chore: update according to review --- .../ui/activity/SsoGrantPermissionActivity.kt | 6 +- .../activity/SsoGrantPermissionViewModel.kt | 62 +++++++++---------- .../davdroid/settings/AccountSettings.kt | 2 +- .../at/bitfire/davdroid/util/UserIdFetcher.kt | 2 +- .../davdroid/util/UserIdFetcherTest.kt | 30 +++++---- 5 files changed, 49 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt index 05e63af61..ba791f31d 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionActivity.kt @@ -41,7 +41,7 @@ class SsoGrantPermissionActivity : AppCompatActivity() { setContentView(R.layout.activity_sso_grant_permission) lifecycleScope.launch { - viewModel.event + viewModel.permissionEvent .flowWithLifecycle( lifecycle = lifecycle, minActiveState = Lifecycle.State.CREATED @@ -53,10 +53,10 @@ class SsoGrantPermissionActivity : AppCompatActivity() { } } - initiateValidation() + validateAccount() } - private fun initiateValidation() { + private fun validateAccount() { val account: Account? = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { intent.getParcelableExtra(Constants.NEXTCLOUD_FILES_ACCOUNT, Account::class.java) diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt index 94f124a8b..51ecf4cf0 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/SsoGrantPermissionViewModel.kt @@ -45,13 +45,13 @@ import javax.inject.Inject class SsoGrantPermissionViewModel @Inject constructor( @ApplicationContext private val context: Context, private val database: AppDatabase, -): ViewModel() { +) : ViewModel() { private val acceptedAccountTypes = listOf(context.getString(R.string.eelo_account_type)) private val acceptedPackages = listOf("foundation.e.notes") - private val _event = MutableSharedFlow() - val event = _event.asSharedFlow() + private val _permissionEvent = MutableSharedFlow() + val permissionEvent = _permissionEvent.asSharedFlow() fun initValidation(callingActivity: ComponentName?, account: Account?) { viewModelScope.launch(Dispatchers.IO) { @@ -60,28 +60,28 @@ class SsoGrantPermissionViewModel @Inject constructor( } } - private suspend fun getCallingPackageName(callingActivity: ComponentName?): String? { - if (callingActivity == null) { - log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Calling Package is null") - _event.emit( - SsoGrantPermissionEvent.PermissionDenied( - errorMessage = Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED - ) + private suspend fun emitPermissionDeniedEvent(message: String) { + _permissionEvent.emit( + SsoGrantPermissionEvent.PermissionDenied( + errorMessage = message ) - return null + ) + } + + private suspend fun getCallingPackageName(callingActivity: ComponentName?): String? { + if (callingActivity != null) { + return callingActivity.packageName } - return callingActivity.packageName + log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Calling Package is null") + emitPermissionDeniedEvent(Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED) + return null } private suspend fun validate(packageName: String?, account: Account?) { if (!isValidRequest(packageName, account)) { log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Invalid request") - _event.emit( - SsoGrantPermissionEvent.PermissionDenied( - errorMessage = Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED - ) - ) + emitPermissionDeniedEvent(Constants.EXCEPTION_ACCOUNT_ACCESS_DECLINED) } val serverUrl = getServerUrl(account!!) ?: return @@ -118,12 +118,9 @@ class SsoGrantPermissionViewModel @Inject constructor( return ocAccount.baseUri.toString() } catch (e: AccountUtils.AccountNotFoundException) { log.log(Level.SEVERE, "SsoGrantPermissionViewModel: Account not found") - _event.emit( - SsoGrantPermissionEvent.PermissionDenied( - errorMessage = Constants.EXCEPTION_ACCOUNT_NOT_FOUND - ) - ) + emitPermissionDeniedEvent(Constants.EXCEPTION_ACCOUNT_NOT_FOUND) } + return null } @@ -139,7 +136,7 @@ class SsoGrantPermissionViewModel @Inject constructor( database.serviceDao().getByAccountName(account.name)?.principal?.toString() ?: return account.name - return UserIdFetcher.retrieveUserId(principalUrl) ?: account.name + return UserIdFetcher.fetch(principalUrl) ?: account.name } private fun saveToken(token: String, accountName: String, packageName: String) { @@ -157,15 +154,16 @@ class SsoGrantPermissionViewModel @Inject constructor( userId: String, serverUrl: String ) { - val result = Bundle() - result.putString(AccountManager.KEY_ACCOUNT_NAME, account.name) - result.putString(AccountManager.KEY_ACCOUNT_TYPE, account.type) - result.putString(AccountManager.KEY_AUTHTOKEN, Constants.NEXTCLOUD_SSO) - result.putString(Constants.SSO_USER_ID, userId) - result.putString(Constants.SSO_TOKEN, token) - result.putString(Constants.SSO_SERVER_URL, serverUrl) - - _event.emit( + val result = Bundle().apply { + putString(AccountManager.KEY_ACCOUNT_NAME, account.name) + putString(AccountManager.KEY_ACCOUNT_TYPE, account.type) + putString(AccountManager.KEY_AUTHTOKEN, Constants.NEXTCLOUD_SSO) + putString(Constants.SSO_USER_ID, userId) + putString(Constants.SSO_TOKEN, token) + putString(Constants.SSO_SERVER_URL, serverUrl) + } + + _permissionEvent.emit( SsoGrantPermissionEvent.PermissionGranted( bundle = result ) 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 613643aba..63d13d23d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -170,7 +170,7 @@ class AccountSettings( } private fun addUserIdToBundle(bundle: Bundle, userName: String?, url: String?) { - val userId = UserIdFetcher.retrieveUserId(url) ?: userName + val userId = UserIdFetcher.fetch(url) ?: userName userId?.let { bundle.putString(NCAccountUtils.Constants.KEY_USER_ID, userId) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt index 184ded411..3152d0163 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt @@ -26,7 +26,7 @@ object UserIdFetcher { * 1. provided url is null * 2. `/users/` part is missing */ - fun retrieveUserId(principalUrl: String?): String? { + fun fetch(principalUrl: String?): String? { if (principalUrl == null) { return null } diff --git a/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt index 1c809efe0..189267e98 100644 --- a/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt +++ b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt @@ -17,46 +17,44 @@ package at.bitfire.davdroid.util import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Test class UserIdFetcherTest { @Test - fun `test retrieveUserId with empty input`() { - val expected = null - val actual = UserIdFetcher.retrieveUserId(null) - assertEquals(expected, actual) + fun `test fetch with empty input`() { + val actual = UserIdFetcher.fetch(null) + assertNull(actual) } @Test - fun `test retrieveUserId with input without 'users' part`() { + fun `test fetch with input without 'users' part`() { val url = "https://murena.io" - val expected = null - val actual = UserIdFetcher.retrieveUserId(url) - assertEquals(expected, actual) + val actual = UserIdFetcher.fetch(url) + assertNull(actual) } @Test - fun `test retrieveUserId with input ends with 'users'`() { + fun `test fetch with input ends with 'users'`() { val baseUrl = "https://murena.io/remote.php/dav/principals/users/" - val expected = null - val actual = UserIdFetcher.retrieveUserId(baseUrl) - assertEquals(expected, actual) + val actual = UserIdFetcher.fetch(baseUrl) + assertNull(actual) } @Test - fun `test retrieveUserId with input with 'users' part`() { + fun `test fetch with input with 'users' part`() { val baseUrl = "https://murena.io/remote.php/dav/principals/users/user@e.email" val expected = "user@e.email" - val actual = UserIdFetcher.retrieveUserId(baseUrl) + val actual = UserIdFetcher.fetch(baseUrl) assertEquals(expected, actual) } @Test - fun `test retrieveUserId with input with 'users' part & ends with slash`() { + fun `test fetch with input with 'users' part & ends with slash`() { val baseUrl = "https://murena.io/remote.php/dav/principals/users/user@e.email/" val expected = "user@e.email" - val actual = UserIdFetcher.retrieveUserId(baseUrl) + val actual = UserIdFetcher.fetch(baseUrl) assertEquals(expected, actual) } } -- GitLab From ec1a3d3a8f80bae9fb7d77dca4b1db5f7e2a4412 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 10 Apr 2024 12:53:12 +0600 Subject: [PATCH 4/5] chore: update according to review --- .../at/bitfire/davdroid/settings/AccountSettings.kt | 10 +++++++++- .../kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt | 9 ++------- .../at/bitfire/davdroid/util/UserIdFetcherTest.kt | 6 ------ 3 files changed, 11 insertions(+), 14 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 63d13d23d..1996677b4 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -170,7 +170,15 @@ class AccountSettings( } private fun addUserIdToBundle(bundle: Bundle, userName: String?, url: String?) { - val userId = UserIdFetcher.fetch(url) ?: userName + var userId: String? = null + + if (!url.isNullOrBlank()) { + userId = UserIdFetcher.fetch(url) + } + + if (userId.isNullOrBlank()) { + userId = userName + } userId?.let { bundle.putString(NCAccountUtils.Constants.KEY_USER_ID, userId) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt index 3152d0163..16c7bf8a9 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/UserIdFetcher.kt @@ -23,14 +23,9 @@ object UserIdFetcher { * example: if the url is: https://abc.com/remote.php/dav/principals/users/xyz/, then this function will return xyz. * * this function will return null, if - * 1. provided url is null - * 2. `/users/` part is missing + * - `/users/` part is missing */ - fun fetch(principalUrl: String?): String? { - if (principalUrl == null) { - return null - } - + fun fetch(principalUrl: String): String? { val usersPart = "/users/" var userId: String? = null diff --git a/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt index 189267e98..467e86774 100644 --- a/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt +++ b/app/src/test/kotlin/at/bitfire/davdroid/util/UserIdFetcherTest.kt @@ -22,12 +22,6 @@ import org.junit.Test class UserIdFetcherTest { - @Test - fun `test fetch with empty input`() { - val actual = UserIdFetcher.fetch(null) - assertNull(actual) - } - @Test fun `test fetch with input without 'users' part`() { val url = "https://murena.io" -- GitLab From ad612bd7d9f061ea1ed34e58a5e6ab0e285c1986 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 15 Apr 2024 08:26:19 +0600 Subject: [PATCH 5/5] chore: update according to review --- .../davdroid/settings/AccountSettings.kt | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 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 1996677b4..787879afe 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt @@ -157,19 +157,27 @@ class AccountSettings( bundle.putString(COOKIE_KEY, cookies) } - var baseUrl : String? = null if (!url.isNullOrEmpty()) { - baseUrl = AccountUtils.getOwnCloudBaseUrl(url) + val baseUrl = AccountUtils.getOwnCloudBaseUrl(url) bundle.putString(NCAccountUtils.Constants.KEY_OC_BASE_URL, baseUrl) } - addUserIdToBundle(bundle, userName, url) - addEmailToBundle(bundle, email, userName) + addUserIdToBundle( + bundle = bundle, + url = url, + userName = userName + ) + + addEmailToBundle( + bundle = bundle, + email = email, + userName = userName + ) return bundle } - private fun addUserIdToBundle(bundle: Bundle, userName: String?, url: String?) { + private fun addUserIdToBundle(bundle: Bundle, url: String?, userName: String?) { var userId: String? = null if (!url.isNullOrBlank()) { -- GitLab