Skip to content

Prevent logout when filling questionnaire #3830

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
FikriMilano wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Prevent logout when filling questionnaire #3830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
926f034
Prevent logout when filling questionnaire
FikriMilano Feb 27, 2026
702fe89
Merge branch 'main' into 3829-prevent-auto-logout-on-token-expiry
dubdabasoduba Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions .../src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ constructor(
private val authConfiguration by lazy { configService.provideAuthConfiguration() }
private var isLoginPageRendered = false

/** When true, suppresses automatic LoginActivity launch (e.g., during questionnaire filling) */
var suppressLoginRedirect = false

@ellykits ellykits Mar 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there another approach that could be used instead of relying on this property of the class to know when the redirect is suppressed?


fun getAccessToken(): String {
val account = findAccount() ?: return ""
val accessToken = accountManager.peekAuthToken(account, AUTH_TOKEN_TYPE) ?: ""
Expand Down Expand Up @@ -118,12 +121,16 @@ constructor(
bundle.containsKey(AccountManager.KEY_INTENT) -> {
val launchIntent = bundle.get(AccountManager.KEY_INTENT) as? Intent

// Deletes session PIN to allow reset
secureSharedPreference.deleteSessionPin()
if (suppressLoginRedirect) {
Timber.w("Login redirect suppressed — questionnaire in progress")
} else {
// Deletes session PIN to allow reset
secureSharedPreference.deleteSessionPin()

if (launchIntent != null && !isLoginPageRendered) {
context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
isLoginPageRendered = true
if (launchIntent != null && !isLoginPageRendered) {
context.startActivity(launchIntent.putExtra(CANCEL_BACKGROUND_SYNC, true))
isLoginPageRendered = true
}
}
Comment on lines 70 to 134

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New suppression behavior (suppressLoginRedirect) changes the auth failure flow but isn’t covered by tests. There are existing Robolectric tests for TokenAuthenticator; please add coverage that verifies: (1) when suppression is enabled and KEY_INTENT is returned, the authenticator does not start the login intent, and (2) when suppression is disabled, the login intent is started as before (and any required cleanup like session PIN deletion still occurs).

Copilot uses AI. Check for mistakes.
Comment on lines 121 to 134

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When suppressLoginRedirect is true, the KEY_INTENT branch skips both launching the login and deleting the session PIN. Previously, session PIN cleanup happened whenever re-authentication was required; leaving the PIN intact after an auth failure can change security/login behavior (e.g., stale PIN still present even though the session is invalid). Consider still clearing the session PIN even when redirect is suppressed, and only suppress the UI navigation portion.

Copilot uses AI. Check for mistakes.
Comment on lines +124 to 134

@ellykits ellykits Mar 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logging part of the if statement is unnecessary.

}
}
Expand Down
1 change: 1 addition & 0 deletions android/engine/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
<string name="percentage" translatable="false">%</string>
<string name="logging_out">Logging out. Please wait…</string>
<string name="session_expired">Session has been expired and must login again</string>

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The session_expired string is grammatically incorrect ("Session has been expired and must login again"). Since this dialog/title is used with the new session-expired messaging, consider updating it to something like "Your session has expired. Please log in again."

Suggested change
<string name="session_expired">Session has been expired and must login again</string>
<string name="session_expired">Your session has expired. Please log in again.</string>

Copilot uses AI. Check for mistakes.
<string name="session_expired_form_saved">Your form has been saved successfully. Your session has expired, please log in again to continue.</string>
<string name="sex">Sex</string>
<string name="age">Age</string>
<string name="dob">DOB</string>
Expand Down
44 changes: 38 additions & 6 deletions android/quest/src/main/java/org/smartregister/fhircore/quest/ui/main/AppMainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ import io.sentry.android.navigation.SentryNavigationListener
import java.time.Instant
import javax.inject.Inject
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.hl7.fhir.r4.model.IdType
import org.hl7.fhir.r4.model.QuestionnaireResponse
import org.smartregister.fhircore.engine.configuration.QuestionnaireConfig
import org.smartregister.fhircore.engine.configuration.app.LocationLogOptions
import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
import org.smartregister.fhircore.engine.datastore.ProtoDataStore
import org.smartregister.fhircore.engine.domain.model.LauncherType
import org.smartregister.fhircore.engine.rulesengine.services.LocationCoordinate
Expand All @@ -58,6 +60,7 @@ import org.smartregister.fhircore.engine.ui.base.AlertDialogue
import org.smartregister.fhircore.engine.ui.base.AlertIntent
import org.smartregister.fhircore.engine.ui.base.BaseMultiLanguageActivity
import org.smartregister.fhircore.engine.util.DispatcherProvider
import org.smartregister.fhircore.engine.util.extension.launchActivityWithNoBackStackHistory
import org.smartregister.fhircore.engine.util.extension.parcelable
import org.smartregister.fhircore.engine.util.extension.serializable
import org.smartregister.fhircore.engine.util.extension.showToast
Expand All @@ -66,6 +69,7 @@ import org.smartregister.fhircore.engine.util.location.PermissionUtils
import org.smartregister.fhircore.quest.R
import org.smartregister.fhircore.quest.event.AppEvent
import org.smartregister.fhircore.quest.event.EventBus
import org.smartregister.fhircore.quest.ui.login.LoginActivity
import org.smartregister.fhircore.quest.ui.questionnaire.QuestionnaireActivity
import org.smartregister.fhircore.quest.ui.shared.ActivityOnResultType
import org.smartregister.fhircore.quest.ui.shared.ON_RESULT_TYPE
Expand All @@ -84,6 +88,8 @@ open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler,

@Inject lateinit var dispatcherProvider: DispatcherProvider

@Inject lateinit var tokenAuthenticator: TokenAuthenticator

val appMainViewModel by viewModels<AppMainViewModel>()
private val sentryNavListener =
SentryNavigationListener(enableNavigationBreadcrumbs = true, enableNavigationTracing = true)
Expand All @@ -110,12 +116,20 @@ open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler,
registerForActivityResult(ActivityResultContracts.StartActivityForResult()) {
activityResult: ActivityResult ->
val onResultType = activityResult.data?.extras?.getString(ON_RESULT_TYPE)
if (
activityResult.resultCode == Activity.RESULT_OK &&
!onResultType.isNullOrBlank() &&
ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE
) {
lifecycleScope.launch { onSubmitQuestionnaire(activityResult) }
lifecycleScope.launch {
if (
activityResult.resultCode == Activity.RESULT_OK &&
!onResultType.isNullOrBlank() &&
ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE
) {
onSubmitQuestionnaire(activityResult)
}

// Check if session expired while questionnaire was open
if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
delay(3000) // Let user see data was saved
showSessionExpiredDialog()
Comment on lines +120 to +131

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new post-questionnaire session-expiry behavior in the startForResult callback isn’t covered by existing AppMainActivityTests. Please add tests to verify the dialog is only shown for questionnaire results (not for LOCATION or other result types), and only when the refresh token is inactive.

Suggested change
if (
activityResult.resultCode == Activity.RESULT_OK &&
!onResultType.isNullOrBlank() &&
ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE
) {
onSubmitQuestionnaire(activityResult)
}
// Check if session expired while questionnaire was open
if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
delay(3000) // Let user see data was saved
showSessionExpiredDialog()
val isQuestionnaireResult =
activityResult.resultCode == Activity.RESULT_OK &&
!onResultType.isNullOrBlank() &&
ActivityOnResultType.valueOf(onResultType) == ActivityOnResultType.QUESTIONNAIRE
if (isQuestionnaireResult) {
onSubmitQuestionnaire(activityResult)
// Check if session expired while questionnaire was open
if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
delay(3000) // Let user see data was saved
showSessionExpiredDialog()
}

Copilot uses AI. Check for mistakes.
Comment on lines +126 to +131

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The session-expiry check runs for every ActivityResult handled by startForResult, including non-questionnaire flows like the location settings intent (showLocationSettingsDialog() uses startForResult.launch(intent)). This can unexpectedly force a login prompt after returning from location settings (or any other future startForResult usage). Consider gating the refresh-token check so it only runs for questionnaire results (e.g., when onResultType == QUESTIONNAIRE, and ideally after a successful save/submit).

Suggested change
}
// Check if session expired while questionnaire was open
if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
delay(3000) // Let user see data was saved
showSessionExpiredDialog()
// Check if session expired while questionnaire was open
if (!tokenAuthenticator.isCurrentRefreshTokenActive()) {
delay(3000) // Let user see data was saved
showSessionExpiredDialog()
}

Copilot uses AI. Check for mistakes.
Comment on lines +129 to +131

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay(3000) is a hard-coded timing value in the result handler. If this delay is meant to be consistent UX behavior, consider extracting it to a named constant (or configurable value) to make the intent clear and avoid magic numbers scattered in callbacks.

Copilot uses AI. Check for mistakes.
}
}
}

Expand Down Expand Up @@ -312,6 +326,24 @@ open class AppMainActivity : BaseMultiLanguageActivity(), QuestionnaireHandler,
}
}

private fun showSessionExpiredDialog() {
AlertDialogue.showAlert(
context = this,
alertIntent = AlertIntent.CONFIRM,
message = getString(org.smartregister.fhircore.engine.R.string.session_expired_form_saved),
title = getString(org.smartregister.fhircore.engine.R.string.session_expired),
confirmButton =
AlertDialogButton(
listener = { dialog ->
dialog.dismiss()
launchActivityWithNoBackStackHistory<LoginActivity>()
},
text = org.smartregister.fhircore.engine.R.string.questionnaire_alert_ack_button_title,
),
cancellable = false,
)
}

private fun overrideOnBackPressListener() {
onBackPressedDispatcher.addCallback(
object : OnBackPressedCallback(true) {
Expand Down
10 changes: 10 additions & 0 deletions .../src/main/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import org.hl7.fhir.r4.model.QuestionnaireResponse
import org.hl7.fhir.r4.model.Resource
import org.smartregister.fhircore.engine.configuration.QuestionnaireConfig
import org.smartregister.fhircore.engine.configuration.app.LocationLogOptions
import org.smartregister.fhircore.engine.data.remote.shared.TokenAuthenticator
import org.smartregister.fhircore.engine.domain.model.ActionParameter
import org.smartregister.fhircore.engine.domain.model.isReadOnly
import org.smartregister.fhircore.engine.domain.model.isSummary
Expand Down Expand Up @@ -79,6 +80,8 @@ class QuestionnaireActivity : BaseMultiLanguageActivity() {
@Inject lateinit var dispatcherProvider: DispatcherProvider

@Inject lateinit var fhirParser: IParser

@Inject lateinit var tokenAuthenticator: TokenAuthenticator
val viewModel by viewModels<QuestionnaireViewModel>()
private lateinit var questionnaireConfig: QuestionnaireConfig
private lateinit var actionParameters: ArrayList<ActionParameter>
Expand Down Expand Up @@ -142,6 +145,8 @@ class QuestionnaireActivity : BaseMultiLanguageActivity() {
return
}

tokenAuthenticator.suppressLoginRedirect = true

viewBinding.questionnaireToolbar.setNavigationIcon(R.drawable.ic_cancel)
viewBinding.questionnaireToolbar.setNavigationOnClickListener { handleBackPress() }
viewBinding.questionnaireTitle.text = questionnaireConfig.title
Expand Down Expand Up @@ -607,6 +612,11 @@ class QuestionnaireActivity : BaseMultiLanguageActivity() {
(supportFragmentManager.findFragmentByTag(QUESTIONNAIRE_FRAGMENT_TAG) as QuestionnaireFragment?)
?.getQuestionnaireResponse()

override fun onDestroy() {
tokenAuthenticator.suppressLoginRedirect = false
super.onDestroy()
Comment on lines 148 to +617

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokenAuthenticator.suppressLoginRedirect is toggled as a global flag from this activity. Please add/extend Robolectric coverage to ensure it is set to true while the questionnaire is active and reliably reset to false when the activity finishes/destroys (including the common finish paths like submit, save-draft, and cancel). This helps prevent the app getting stuck in a permanently suppressed state if lifecycle paths change.

Copilot uses AI. Check for mistakes.
}

companion object {

const val QUESTIONNAIRE_FRAGMENT_TAG = "questionnaireFragment"
Expand Down
Loading