[MS-1352] MFID: Accessing a correct scanned credential field to build an Identification response

[alexandr-simprints]

JIRA ticket
Will be released in: 2025.4.1

Root cause analysis (for bugfixes only)

First known affected version: 2025.4.1 (during a smoke test)

• Scan a credential
• Edit said credential from, let's say, 12345678 to 00000000
• The identification response will still contain 12345678 instead of the expected edited value (00000000)

Notable changes

• Accessing correct field from the ScannedCredential response

Testing guidance

• Scan a credential
• Edit said credential from, let's say, 12345678 to 00000000
• Verify that the Identification response contains 00000000 (not 12345678)

Additional work checklist

• Effect on other features and security has been considered
• Test cases in Testiny are up to date (or ticket created)

[Copilot]

Pull request overview

This pull request fixes a bug in the Multi-Factor ID (MFID) feature where the identification response was using the original scanned credential value instead of the edited value when a user modified a scanned credential.

Changes:

• Fixed CreateIdentifyResponseUseCase to use the credential field (edited, tokenized value) instead of scannedValue (original scanned value)
• Added TokenizationProcessor dependency to decrypt the credential before including it in the response
• Updated all test signatures to include the new project and tokenizationProcessor parameters

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
feature/orchestrator/src/main/java/com/simprints/feature/orchestrator/usecases/response/CreateIdentifyResponseUseCase.kt	Core bug fix: changed from using scannedValue to decrypting the credential field; added TokenizationProcessor dependency
feature/orchestrator/src/main/java/com/simprints/feature/orchestrator/usecases/response/AppResponseBuilderUseCase.kt	Thread the project parameter through to CreateIdentifyResponseUseCase
feature/orchestrator/src/test/java/com/simprints/feature/orchestrator/usecases/response/CreateIdentifyResponseUseCaseTest.kt	Updated all test method signatures to include new project parameter
feature/orchestrator/src/test/java/com/simprints/feature/orchestrator/usecases/response/AppResponseBuilderUseCaseTest.kt	Updated mock expectations to include the new project parameter

[luhmirin-s]

I am very confused - the PR description is about returning the edited value but the changes are only about tokenising the previously provided value.

[alexandr-simprints]

Previously it was accessing the initially captured value, which is unencrypted: ScannedCredential::scannedValue

Not it is accessing the final credential value, which is encrypted: ScannedCredential::credential

See here

private fun ScannedCredential.toAppExternalCredential( 
    tokenizationProcessor: TokenizationProcessor,
    project: Project?,
): AppExternalCredential? {
    val decryptedValue = tokenizationProcessor.decrypt(
        encrypted = credential, // <--- Here
    )
    // [...]
    return AppExternalCredential(
        id = credentialScanId,
        value = decryptedValue,
        type = credentialType,
    )

[BurningAXE]

I was confused by the same. Why aren't we encrypting scannedValue, too? It will be the actual credential in > 99% of cases and having it saved as plain string kind of makes credential encryption pointless.

[alexandr-simprints]

The purpose of the scanned value is for evaluation of how off was scanned value from the final value (stored as credential), see here and here

I see why it might be confusing, I have plans to refactor the returning types a bit for easier separation of concerns. Currently, the ScannedCredential is assigned too much responsibilities that arose as a consequence of the changed requirements

[BurningAXE]

I can see why we'd want both the scanned and the final values. However, encrypting just the final one doesn't make much sense. Is there any reason for this discrepancy?

[luhmirin-s]

IIRC, we have one event that requires both so that we can see how often attendants need to edit ORC results.

[BurningAXE]

My question is not why we keep both but why we encrypt only one of them? They are supposed to be the same values in > 99% of cases.

[luhmirin-s]

FSR :D

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud