From c53d659b68a7e024bcd0fd93eb1d5b626136f45b Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 00:14:09 +0530 Subject: [PATCH 1/6] App lounge: (issue_5168) modify DataStoreModule.destroyCredentials() to not remove USERTYPE --- .../foundation/e/apps/utils/modules/DataStoreModule.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt b/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt index 33e3dc1ce..4cc3c8892 100644 --- a/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt +++ b/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt @@ -64,12 +64,18 @@ class DataStoreModule @Inject constructor( } } + /** + * Destroy auth credentials if they are no longer valid. + * + * Modification for issue: https://gitlab.e.foundation/e/backlog/-/issues/5168 + * Previously this method would also remove [USERTYPE]. + * But now it is only to be done from [saveUserType] passing [User.UNAVAILABLE] when clicking logout. + */ suspend fun destroyCredentials() { context.dataStore.edit { it.remove(AUTHDATA) it.remove(EMAIL) it.remove(OAUTHTOKEN) - it.remove(USERTYPE) } } -- GitLab From 79b0ec815a93d0632204e68a79f845e8acaa0693 Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 03:32:37 +0530 Subject: [PATCH 2/6] App lounge: (issue_5168) Move code block that generate auth data in MainActivity to a separate function --- .../java/foundation/e/apps/MainActivity.kt | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/MainActivity.kt b/app/src/main/java/foundation/e/apps/MainActivity.kt index 5c1c7fcd0..fb2c11e31 100644 --- a/app/src/main/java/foundation/e/apps/MainActivity.kt +++ b/app/src/main/java/foundation/e/apps/MainActivity.kt @@ -83,6 +83,28 @@ class MainActivity : AppCompatActivity() { } } + fun generateAuthDataBasedOnUserType(user: String) { + if (user.isNotBlank() && viewModel.tocStatus.value == true) { + when (User.valueOf(user)) { + User.ANONYMOUS -> { + if (viewModel.authDataJson.value.isNullOrEmpty() && !viewModel.authRequestRunning) { + Log.d(TAG, "Fetching new authentication data") + viewModel.getAuthData() + } + } + User.UNAVAILABLE -> { + viewModel.destroyCredentials() + } + User.GOOGLE -> { + if (viewModel.authData.value == null && !viewModel.authRequestRunning) { + Log.d(TAG, "Fetching new authentication data") + signInViewModel.fetchAuthData() + } + } + } + } + } + viewModel.internetConnection.observe(this) { isInternetAvailable -> hasInternet = isInternetAvailable if (isInternetAvailable) { @@ -90,25 +112,7 @@ class MainActivity : AppCompatActivity() { binding.fragment.visibility = View.VISIBLE viewModel.userType.observe(this) { user -> - if (user.isNotBlank() && viewModel.tocStatus.value == true) { - when (User.valueOf(user)) { - User.ANONYMOUS -> { - if (viewModel.authDataJson.value.isNullOrEmpty() && !viewModel.authRequestRunning) { - Log.d(TAG, "Fetching new authentication data") - viewModel.getAuthData() - } - } - User.UNAVAILABLE -> { - viewModel.destroyCredentials() - } - User.GOOGLE -> { - if (viewModel.authData.value == null && !viewModel.authRequestRunning) { - Log.d(TAG, "Fetching new authentication data") - signInViewModel.fetchAuthData() - } - } - } - } + generateAuthDataBasedOnUserType(user) } signInViewModel.authLiveData.observe(this) { -- GitLab From 86f9be0ff375d5947b62aaa65ae0001d69ac4351 Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 03:34:36 +0530 Subject: [PATCH 3/6] App lounge: (issue_5168) on authValidity being false, directly call dataStoreModule.destroyCredentials (not via viewmodel), and generate new auth data. --- .../java/foundation/e/apps/MainActivity.kt | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/MainActivity.kt b/app/src/main/java/foundation/e/apps/MainActivity.kt index fb2c11e31..050ca34e0 100644 --- a/app/src/main/java/foundation/e/apps/MainActivity.kt +++ b/app/src/main/java/foundation/e/apps/MainActivity.kt @@ -46,15 +46,20 @@ import foundation.e.apps.updates.UpdatesNotifier import foundation.e.apps.utils.enums.Status import foundation.e.apps.utils.enums.User import foundation.e.apps.utils.modules.CommonUtilsModule +import foundation.e.apps.utils.modules.DataStoreModule import kotlinx.coroutines.launch import java.io.File import java.util.UUID +import javax.inject.Inject @AndroidEntryPoint class MainActivity : AppCompatActivity() { private lateinit var binding: ActivityMainBinding private val TAG = MainActivity::class.java.simpleName + @Inject + lateinit var dataStoreModule: DataStoreModule + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) binding = ActivityMainBinding.inflate(layoutInflater) @@ -130,11 +135,33 @@ class MainActivity : AppCompatActivity() { } viewModel.authValidity.observe(this) { - if (it != true) { - Log.d(TAG, "Authentication data validation failed!") - viewModel.destroyCredentials() - } else { - Log.d(TAG, "Authentication data is valid!") + lifecycleScope.launch { + if (it != true) { + Log.d(TAG, "Authentication data validation failed!") + /* + * Issue: https://gitlab.e.foundation/e/backlog/-/issues/5168 + * + * Previously we were using viewmodel.destroyCredentials(), without + * a coroutine scope. + * + * If auth data is invalid, we need old credentials to be removed followed by + * generation of new auth data. If we use via viewmodel, the two jobs do not + * occur sequentially. Hence we run the suspend function directly + * inside a coroutine scope. + * + * ALso now destroyCredentials() no longer removes the user type + * (i.e. Google login or Anonymous). User type data is only removed on logout. + * New auth data is generated without prompting the user to choose Google login + * or Anonymous; which is the purpose of the issue filed above. + */ + dataStoreModule.destroyCredentials() + dataStoreModule.userType.collect { user -> + Log.d(TAG, "Regenerating auth data for user type: $user") + generateAuthDataBasedOnUserType(user) + } + } else { + Log.d(TAG, "Authentication data is valid!") + } } } -- GitLab From 958cb182ee7bff76ee2d6a4bdbe3efd3792c526d Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 02:51:49 +0530 Subject: [PATCH 4/6] App lounge: (issue_5168) create function clearUserType() in DataStoreModule --- .../foundation/e/apps/utils/modules/DataStoreModule.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt b/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt index 4cc3c8892..bccf26392 100644 --- a/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt +++ b/app/src/main/java/foundation/e/apps/utils/modules/DataStoreModule.kt @@ -69,7 +69,7 @@ class DataStoreModule @Inject constructor( * * Modification for issue: https://gitlab.e.foundation/e/backlog/-/issues/5168 * Previously this method would also remove [USERTYPE]. - * But now it is only to be done from [saveUserType] passing [User.UNAVAILABLE] when clicking logout. + * To clear this value, call [clearUserType]. */ suspend fun destroyCredentials() { context.dataStore.edit { @@ -79,6 +79,12 @@ class DataStoreModule @Inject constructor( } } + suspend fun clearUserType() { + context.dataStore.edit { + it.remove(USERTYPE) + } + } + /** * TOC status */ -- GitLab From c9aa357c69fb5d39f3a6c0098923313aa4fad1ea Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 03:36:49 +0530 Subject: [PATCH 5/6] App lounge: (issue_5168) modify MainActivity to clear user type if auth data is invalid and user type is not ANONYMOUS --- .../java/foundation/e/apps/MainActivity.kt | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/MainActivity.kt b/app/src/main/java/foundation/e/apps/MainActivity.kt index 050ca34e0..2580fd6b5 100644 --- a/app/src/main/java/foundation/e/apps/MainActivity.kt +++ b/app/src/main/java/foundation/e/apps/MainActivity.kt @@ -149,15 +149,23 @@ class MainActivity : AppCompatActivity() { * occur sequentially. Hence we run the suspend function directly * inside a coroutine scope. * - * ALso now destroyCredentials() no longer removes the user type - * (i.e. Google login or Anonymous). User type data is only removed on logout. - * New auth data is generated without prompting the user to choose Google login - * or Anonymous; which is the purpose of the issue filed above. + * Now destroyCredentials() no longer removes the user type from data store. + * (i.e. Google login or Anonymous). + * - If the type is User.ANONYMOUS then we do not prompt the user to login again, + * we directly generate new auth data; which is the main Gitlab issue described above. + * - If not anonymous user, i.e. type is User.GOOGLE, in that case we clear + * the USERTYPE value. This causes HomeFragment.onTosAccepted() to open + * SignInFragment as we need fresh login from the user. */ dataStoreModule.destroyCredentials() dataStoreModule.userType.collect { user -> - Log.d(TAG, "Regenerating auth data for user type: $user") - generateAuthDataBasedOnUserType(user) + if (!user.isBlank() && User.valueOf(user) == User.ANONYMOUS) { + Log.d(TAG, "Regenerating auth data for Anonymous user") + generateAuthDataBasedOnUserType(user) + } else { + Log.d(TAG, "Ask Google user to log in again") + dataStoreModule.clearUserType() + } } } else { Log.d(TAG, "Authentication data is valid!") -- GitLab From 5b0ab2c8ea17003e05173cef235da06329e01698 Mon Sep 17 00:00:00 2001 From: SayantanRC Date: Wed, 13 Apr 2022 18:34:38 +0530 Subject: [PATCH 6/6] App lounge: (issue_5168) remove dataStoreModule from MainActivity and move logic to MainActivityViewModel --- .../java/foundation/e/apps/MainActivity.kt | 46 +++---------------- .../e/apps/MainActivityViewModel.kt | 25 +++++++++- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/MainActivity.kt b/app/src/main/java/foundation/e/apps/MainActivity.kt index 2580fd6b5..22e12c867 100644 --- a/app/src/main/java/foundation/e/apps/MainActivity.kt +++ b/app/src/main/java/foundation/e/apps/MainActivity.kt @@ -46,7 +46,6 @@ import foundation.e.apps.updates.UpdatesNotifier import foundation.e.apps.utils.enums.Status import foundation.e.apps.utils.enums.User import foundation.e.apps.utils.modules.CommonUtilsModule -import foundation.e.apps.utils.modules.DataStoreModule import kotlinx.coroutines.launch import java.io.File import java.util.UUID @@ -57,9 +56,6 @@ class MainActivity : AppCompatActivity() { private lateinit var binding: ActivityMainBinding private val TAG = MainActivity::class.java.simpleName - @Inject - lateinit var dataStoreModule: DataStoreModule - override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) binding = ActivityMainBinding.inflate(layoutInflater) @@ -98,7 +94,7 @@ class MainActivity : AppCompatActivity() { } } User.UNAVAILABLE -> { - viewModel.destroyCredentials() + viewModel.destroyCredentials(null) } User.GOOGLE -> { if (viewModel.authData.value == null && !viewModel.authRequestRunning) { @@ -135,41 +131,13 @@ class MainActivity : AppCompatActivity() { } viewModel.authValidity.observe(this) { - lifecycleScope.launch { - if (it != true) { - Log.d(TAG, "Authentication data validation failed!") - /* - * Issue: https://gitlab.e.foundation/e/backlog/-/issues/5168 - * - * Previously we were using viewmodel.destroyCredentials(), without - * a coroutine scope. - * - * If auth data is invalid, we need old credentials to be removed followed by - * generation of new auth data. If we use via viewmodel, the two jobs do not - * occur sequentially. Hence we run the suspend function directly - * inside a coroutine scope. - * - * Now destroyCredentials() no longer removes the user type from data store. - * (i.e. Google login or Anonymous). - * - If the type is User.ANONYMOUS then we do not prompt the user to login again, - * we directly generate new auth data; which is the main Gitlab issue described above. - * - If not anonymous user, i.e. type is User.GOOGLE, in that case we clear - * the USERTYPE value. This causes HomeFragment.onTosAccepted() to open - * SignInFragment as we need fresh login from the user. - */ - dataStoreModule.destroyCredentials() - dataStoreModule.userType.collect { user -> - if (!user.isBlank() && User.valueOf(user) == User.ANONYMOUS) { - Log.d(TAG, "Regenerating auth data for Anonymous user") - generateAuthDataBasedOnUserType(user) - } else { - Log.d(TAG, "Ask Google user to log in again") - dataStoreModule.clearUserType() - } - } - } else { - Log.d(TAG, "Authentication data is valid!") + if (it != true) { + Log.d(TAG, "Authentication data validation failed!") + viewModel.destroyCredentials { user -> + generateAuthDataBasedOnUserType(user) } + } else { + Log.d(TAG, "Authentication data is valid!") } } diff --git a/app/src/main/java/foundation/e/apps/MainActivityViewModel.kt b/app/src/main/java/foundation/e/apps/MainActivityViewModel.kt index e6b4c91fa..d224adc73 100644 --- a/app/src/main/java/foundation/e/apps/MainActivityViewModel.kt +++ b/app/src/main/java/foundation/e/apps/MainActivityViewModel.kt @@ -42,6 +42,7 @@ import foundation.e.apps.manager.fused.FusedManagerRepository import foundation.e.apps.utils.enums.Origin import foundation.e.apps.utils.enums.Status import foundation.e.apps.utils.enums.Type +import foundation.e.apps.utils.enums.User import foundation.e.apps.utils.modules.DataStoreModule import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -99,9 +100,31 @@ class MainActivityViewModel @Inject constructor( _authData.value = authData } - fun destroyCredentials() { + fun destroyCredentials(regenerateFunction: ((user: String) -> Unit)?) { viewModelScope.launch { + /* + * Issue: https://gitlab.e.foundation/e/backlog/-/issues/5168 + * + * Now destroyCredentials() no longer removes the user type from data store. + * (i.e. Google login or Anonymous). + * - If the type is User.ANONYMOUS then we do not prompt the user to login again, + * we directly generate new auth data; which is the main Gitlab issue described above. + * - If not anonymous user, i.e. type is User.GOOGLE, in that case we clear + * the USERTYPE value. This causes HomeFragment.onTosAccepted() to open + * SignInFragment as we need fresh login from the user. + */ dataStoreModule.destroyCredentials() + if (regenerateFunction != null) { + dataStoreModule.userType.collect { user -> + if (!user.isBlank() && User.valueOf(user) == User.ANONYMOUS) { + Log.d(TAG, "Regenerating auth data for Anonymous user") + regenerateFunction(user) + } else { + Log.d(TAG, "Ask Google user to log in again") + dataStoreModule.clearUserType() + } + } + } } } -- GitLab