Skip to content

[MS-1214] Validate that Confirmation GUID was in Identification results #1433

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

Merged
BurningAXE merged 2 commits into from
Nov 6, 2025
Merged

[MS-1214] Validate that Confirmation GUID was in Identification results #1433

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
0727c8b
[MS-1214] Validate that Confirmation GUID was in Identification results
BurningAXE Oct 27, 2025
78f7408
[MS-1214] Add a flag to allow skipping extra Confirmation GUID valida…
BurningAXE Nov 5, 2025
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
11 changes: 7 additions & 4 deletions ...api/src/main/java/com/simprints/feature/clientapi/mappers/request/IntentToActionMapper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ import com.simprints.feature.clientapi.models.CommCareConstants
import com.simprints.feature.clientapi.models.LibSimprintsConstants
import com.simprints.feature.clientapi.models.OdkConstants
import com.simprints.feature.clientapi.usecases.GetCurrentSessionIdUseCase
import com.simprints.feature.clientapi.usecases.SessionHasIdentificationCallbackUseCase
import com.simprints.infra.config.store.models.Project
import com.simprints.infra.config.store.tokenization.TokenizationProcessor
import com.simprints.infra.config.sync.ConfigManager
import com.simprints.infra.events.EventRepository
import com.simprints.infra.orchestration.data.ActionConstants
import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
Expand All @@ -37,9 +38,10 @@ import javax.inject.Inject

internal class IntentToActionMapper @Inject constructor(
private val getCurrentSessionId: GetCurrentSessionIdUseCase,
private val sessionHasIdentificationCallback: SessionHasIdentificationCallbackUseCase,
private val tokenizationProcessor: TokenizationProcessor,
private val timeHelper: TimeHelper,
private val eventRepository: EventRepository,
private val configManager: ConfigManager,
) {
suspend operator fun invoke(
action: String,
Expand Down Expand Up @@ -245,7 +247,7 @@ internal class IntentToActionMapper @Inject constructor(
validator = EnrolLastBiometricsValidator(
extractor = extractor,
currentSessionId = getCurrentSessionId(),
sessionHasIdentificationCallback = sessionHasIdentificationCallback(extractor.getSessionId()),
eventRepository = eventRepository,
),
)

Expand All @@ -261,7 +263,8 @@ internal class IntentToActionMapper @Inject constructor(
validator = ConfirmIdentityValidator(
extractor = extractor,
currentSessionId = getCurrentSessionId(),
sessionHasIdentificationCallback = sessionHasIdentificationCallback(extractor.getSessionId()),
eventRepository = eventRepository,
configManager = configManager,
),
)
}
2 changes: 1 addition & 1 deletion ...ain/java/com/simprints/feature/clientapi/mappers/request/builders/ActionRequestBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal abstract class ActionRequestBuilder(
value
}

fun build(): ActionRequest {
suspend fun build(): ActionRequest {
validator.validate()
return buildAction().run(::encryptIfNecessary)
}
Expand Down
51 changes: 45 additions & 6 deletions ...va/com/simprints/feature/clientapi/mappers/request/validators/ConfirmIdentityValidator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,25 @@ package com.simprints.feature.clientapi.mappers.request.validators
import com.simprints.feature.clientapi.exceptions.InvalidRequestException
import com.simprints.feature.clientapi.mappers.request.extractors.ConfirmIdentityRequestExtractor
import com.simprints.feature.clientapi.models.ClientApiError
import com.simprints.infra.config.store.models.experimental
import com.simprints.infra.config.sync.ConfigManager
import com.simprints.infra.events.EventRepository
import com.simprints.infra.events.event.domain.models.callback.IdentificationCallbackEvent
import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SESSION
import com.simprints.infra.logging.Simber

internal class ConfirmIdentityValidator(
private val extractor: ConfirmIdentityRequestExtractor,
private val currentSessionId: String,
private val sessionHasIdentificationCallback: Boolean,
private val eventRepository: EventRepository,
private val configManager: ConfigManager,
) : RequestActionValidator(extractor) {
override fun validate() {
private var identificationEvent: IdentificationCallbackEvent? = null
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

Using mutable state (var) in a validator class can lead to subtle bugs if the validator is reused. Consider passing identificationEvent as a parameter to validateSelectedGuid instead of storing it as mutable state. This would make the code more functional and thread-safe.

Copilot uses AI. Check for mistakes.

override suspend fun validate() {
validateProjectId()
validateSessionId(extractor.getSessionId())
validateSessionEvents()
validateSessionEvents(extractor.getSessionId())
validateSelectedGuid(extractor.getSelectedGuid())
}

Expand All @@ -28,18 +35,50 @@ internal class ConfirmIdentityValidator(
}
}

private fun validateSessionEvents() {
if (!sessionHasIdentificationCallback) {
private suspend fun validateSessionEvents(sessionId: String) {
identificationEvent = eventRepository
.getEventsFromScope(sessionId)
.filterIsInstance<IdentificationCallbackEvent>()
.lastOrNull()

if (identificationEvent == null) {
throw InvalidRequestException(
"Calling app wants to confirm identity, but the session doesn't have an identification callback event.",
ClientApiError.INVALID_SESSION_ID,
)
}
}

private fun validateSelectedGuid(selectedId: String) {
private suspend fun validateSelectedGuid(selectedId: String) {
if (selectedId.isBlank()) {
throw InvalidRequestException("Missing Selected GUID", ClientApiError.INVALID_SELECTED_ID)
}

// Allow 'NONE_SELECTED' as a special case to indicate no selection
if (selectedId.equals("NONE_SELECTED", ignoreCase = true)) {
return
}

// Skip further validation if skip flag is enabled
if (configManager.getProjectConfiguration().experimental().allowConfirmingGuidsNotInCallback) {
return
}

val validGuids = identificationEvent?.payload?.scores?.map { it.guid } ?: emptyList()

if (validGuids.isEmpty()) {
throw InvalidRequestException(
"No identification results found in session",
ClientApiError.INVALID_SELECTED_ID,
)
}

if (!validGuids.contains(selectedId)) {
Simber.i("Selected GUID '$selectedId' not found in identification results: $validGuids", tag = SESSION)
throw InvalidRequestException(
"Selected GUID was not part of the identification results",
ClientApiError.INVALID_SELECTED_ID,
)
}
}
}
16 changes: 11 additions & 5 deletions ...om/simprints/feature/clientapi/mappers/request/validators/EnrolLastBiometricsValidator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ package com.simprints.feature.clientapi.mappers.request.validators
import com.simprints.feature.clientapi.exceptions.InvalidRequestException
import com.simprints.feature.clientapi.mappers.request.extractors.EnrolLastBiometricsRequestExtractor
import com.simprints.feature.clientapi.models.ClientApiError
import com.simprints.infra.events.EventRepository
import com.simprints.infra.events.event.domain.models.callback.IdentificationCallbackEvent
import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SESSION
import com.simprints.infra.logging.Simber

internal class EnrolLastBiometricsValidator(
private val extractor: EnrolLastBiometricsRequestExtractor,
private val currentSessionId: String,
private val sessionHasIdentificationCallback: Boolean,
private val eventRepository: EventRepository,
) : RequestActionValidator(extractor) {
override fun validate() {
override suspend fun validate() {
super.validate()
validateSessionId(extractor.getSessionId())
validateSessionEvents()
validateSessionEvents(extractor.getSessionId())
}

private fun validateSessionId(sessionId: String) {
Expand All @@ -27,8 +29,12 @@ internal class EnrolLastBiometricsValidator(
}
}

private fun validateSessionEvents() {
if (!sessionHasIdentificationCallback) {
private suspend fun validateSessionEvents(sessionId: String) {
val hasIdentificationCallback = eventRepository
.getEventsFromScope(sessionId)
.any { it is IdentificationCallbackEvent }

if (!hasIdentificationCallback) {
throw InvalidRequestException(
"Calling app wants to enrol last biometrics, but the session doesn't have an identification callback event.",
ClientApiError.INVALID_STATE_FOR_INTENT_ACTION,
Expand Down
2 changes: 1 addition & 1 deletion ...java/com/simprints/feature/clientapi/mappers/request/validators/RequestActionValidator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import com.simprints.feature.clientapi.models.ClientApiError
internal abstract class RequestActionValidator(
private val extractor: ActionRequestExtractor,
) {
open fun validate() {
open suspend fun validate() {
validateProjectId()
validateUserId()
validateModuleId()
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/com/simprints/feature/clientapi/mappers/request/validators/VerifyValidator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import com.simprints.feature.clientapi.models.ClientApiError
internal class VerifyValidator(
private val extractor: VerifyRequestExtractor,
) : RequestActionValidator(extractor) {
override fun validate() {
override suspend fun validate() {
super.validate()
validateVerifyGuid(extractor.getVerifyGuid())
}
Expand Down
13 changes: 0 additions & 13 deletions .../java/com/simprints/feature/clientapi/usecases/SessionHasIdentificationCallbackUseCase.kt
View file
Open in desktop

This file was deleted.

27 changes: 21 additions & 6 deletions ...src/test/java/com/simprints/feature/clientapi/mappers/request/IntentToActionMapperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import com.simprints.core.tools.time.Timestamp
import com.simprints.feature.clientapi.exceptions.InvalidRequestException
import com.simprints.feature.clientapi.models.ClientApiConstants
import com.simprints.feature.clientapi.usecases.GetCurrentSessionIdUseCase
import com.simprints.feature.clientapi.usecases.SessionHasIdentificationCallbackUseCase
import com.simprints.infra.config.store.tokenization.TokenizationProcessor
import com.simprints.infra.events.event.domain.models.callback.IdentificationCallbackEvent
import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.libsimprints.Constants.SIMPRINTS_LIB_VERSION
import com.simprints.libsimprints.Constants.SIMPRINTS_MODULE_ID
Expand All @@ -22,6 +22,7 @@ import io.mockk.MockKAnnotations
import io.mockk.coEvery
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Rule
Expand All @@ -35,30 +36,44 @@ class IntentToActionMapperTest {
@MockK
private lateinit var getCurrentSessionIdUseCase: GetCurrentSessionIdUseCase

@MockK
private lateinit var sessionHasIdentificationCallback: SessionHasIdentificationCallbackUseCase

@MockK
private lateinit var tokenizationProcessor: TokenizationProcessor

@MockK
private lateinit var timeHelper: TimeHelper

@MockK
private lateinit var eventRepository: com.simprints.infra.events.EventRepository

@MockK
private lateinit var configManager: com.simprints.infra.config.sync.ConfigManager

private lateinit var mapper: IntentToActionMapper

@Before
fun setUp() {
MockKAnnotations.init(this, relaxed = true)

coEvery { getCurrentSessionIdUseCase.invoke() } returns SESSION_ID
coEvery { sessionHasIdentificationCallback.invoke(any()) } returns true
every { timeHelper.now() } returns Timestamp(0L)
coEvery { eventRepository.getEventsFromScope(any()) } returns listOf(
mockk<IdentificationCallbackEvent> {
every { payload } returns mockk {
every { scores } returns listOf(
mockk {
every { guid } returns SESSION_ID
},
)
}
},
)

mapper = IntentToActionMapper(
getCurrentSessionIdUseCase,
sessionHasIdentificationCallback,
tokenizationProcessor,
timeHelper,
eventRepository,
configManager,
)
}

Expand Down
3 changes: 2 additions & 1 deletion ...simprints/feature/clientapi/mappers/request/builders/ConfirmIdentifyRequestBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Test

internal class ConfirmIdentifyRequestBuilderTest {
@Test
fun `ConfirmActionRequest should contain mandatory fields`() {
fun `ConfirmActionRequest should contain mandatory fields`() = runTest {
val extractor = ConfirmIdentityActionFactory.getMockExtractor()
val validator = ConfirmIdentityActionFactory.getValidator(extractor)
val tokenizationProcessor = mockk<TokenizationProcessor>()
Expand Down
3 changes: 2 additions & 1 deletion ...prints/feature/clientapi/mappers/request/builders/EnrolLastBiometricRequestBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Test

internal class EnrolLastBiometricRequestBuilderTest {
@Test
fun `EnrolLastBiometricActionRequest should contain mandatory fields`() {
fun `EnrolLastBiometricActionRequest should contain mandatory fields`() = runTest {
val extractor = EnrolLastBiometricsActionFactory.getMockExtractor()
val validator = EnrolLastBiometricsActionFactory.getValidator(extractor)
val tokenizationProcessor = mockk<TokenizationProcessor>()
Expand Down
3 changes: 2 additions & 1 deletion .../java/com/simprints/feature/clientapi/mappers/request/builders/EnrolRequestBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Test

internal class EnrolRequestBuilderTest {
@Test
fun `EnrolActionRequest should contain mandatory fields`() {
fun `EnrolActionRequest should contain mandatory fields`() = runTest {
val extractor = EnrolActionFactory.getMockExtractor()
val validator = EnrolActionFactory.getValidator(extractor)
val tokenizationProcessor = mockk<TokenizationProcessor>()
Expand Down
3 changes: 2 additions & 1 deletion ...va/com/simprints/feature/clientapi/mappers/request/builders/IdentifyRequestBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Test

internal class IdentifyRequestBuilderTest {
@Test
fun `IdentifyActionRequest should contain mandatory fields`() {
fun `IdentifyActionRequest should contain mandatory fields`() = runTest {
val extractor = IdentifyRequestActionFactory.getMockExtractor()
val validator = IdentifyRequestActionFactory.getValidator(extractor)
val tokenizationProcessor = mockk<TokenizationProcessor>()
Expand Down
3 changes: 2 additions & 1 deletion ...java/com/simprints/feature/clientapi/mappers/request/builders/VerifyRequestBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.test.runTest
import org.junit.Test

internal class VerifyRequestBuilderTest {
@Test
fun `VerifyActionRequest should contain mandatory fields`() {
fun `VerifyActionRequest should contain mandatory fields`() = runTest {
val extractor = VerifyActionFactory.getMockExtractor()
val validator = VerifyActionFactory.getValidator(extractor)
val tokenizationProcessor = mockk<TokenizationProcessor>()
Expand Down
33 changes: 28 additions & 5 deletions ...prints/feature/clientapi/mappers/request/requestFactories/ConfirmIdentityActionFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ import com.simprints.feature.clientapi.mappers.request.builders.ConfirmIdentifyR
import com.simprints.feature.clientapi.mappers.request.extractors.ActionRequestExtractor
import com.simprints.feature.clientapi.mappers.request.extractors.ConfirmIdentityRequestExtractor
import com.simprints.feature.clientapi.mappers.request.validators.ConfirmIdentityValidator
import com.simprints.infra.config.sync.ConfigManager
import com.simprints.infra.events.EventRepository
import com.simprints.infra.events.event.domain.models.callback.IdentificationCallbackEvent
import com.simprints.infra.orchestration.data.ActionConstants
import com.simprints.infra.orchestration.data.ActionRequest
import com.simprints.infra.orchestration.data.ActionRequestIdentifier
import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk

Expand All @@ -30,11 +34,30 @@ internal object ConfirmIdentityActionFactory : RequestActionFactory() {
unknownExtras = emptyMap(),
)

override fun getValidator(extractor: ActionRequestExtractor): ConfirmIdentityValidator = ConfirmIdentityValidator(
extractor as ConfirmIdentityRequestExtractor,
MOCK_SESSION_ID,
true,
)
override fun getValidator(extractor: ActionRequestExtractor): ConfirmIdentityValidator {
val mockEventRepository = mockk<EventRepository>()
// Return a valid identification callback event with the selected GUID
coEvery { mockEventRepository.getEventsFromScope(any()) } returns listOf(
mockk<IdentificationCallbackEvent> {
every { payload } returns mockk {
every { scores } returns listOf(
mockk {
every { guid } returns MOCK_SELECTED_GUID
},
)
}
},
)

val mockConfigManager = mockk<ConfigManager>(relaxed = true)

return ConfirmIdentityValidator(
extractor as ConfirmIdentityRequestExtractor,
MOCK_SESSION_ID,
mockEventRepository,
configManager = mockConfigManager,
)
}

override fun getBuilder(extractor: ActionRequestExtractor): ConfirmIdentifyRequestBuilder = ConfirmIdentifyRequestBuilder(
actionIdentifier = getIdentifier(),
Expand Down
Loading