Skip to content

[MOSIP] SDK-based biometric quality evaluation with SBI fallback #784

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
Hendrixx-RE wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[MOSIP] SDK-based biometric quality evaluation with SBI fallback #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions ...ation/registration-services/src/main/java/io/mosip/registration/constants/AuditEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@ public enum AuditEvent {
MDM_NO_DEVICE_AVAILABLE("REG-EVT-087", USER_EVENT.getCode(), "DEVICE_NOT_FOUND", "No device is available"),
MDM_DEVICE_FOUND("REG-EVT-088", USER_EVENT.getCode(), "MDM_DEVICE_FOUND", "Device is found"),

// SDK Quality Evaluation
SDK_QUALITY_SUCCESS("REG-EVT-104", SYSTEM_EVENT.getCode(), "SDK_QUALITY_SUCCESS", "SDK biometric quality evaluation succeeded"),
SDK_QUALITY_FAILED("REG-EVT-105", SYSTEM_EVENT.getCode(), "SDK_QUALITY_FAILED", "SDK biometric quality evaluation failed, registration blocked"),
SDK_FALLBACK_TO_SBI("REG-EVT-106", SYSTEM_EVENT.getCode(), "SDK_FALLBACK_TO_SBI", "SDK unavailable, fell back to SBI quality score"),
SDK_QUALITY_DISABLED("REG-EVT-107", SYSTEM_EVENT.getCode(), "SDK_QUALITY_DISABLED", "SDK quality evaluation disabled, using SBI score"),
NO_VALID_QUALITY_SCORE("REG-EVT-108", SYSTEM_EVENT.getCode(), "NO_VALID_QUALITY_SCORE", "No valid quality score from SDK or SBI, registration blocked"),

REG_DOC_SCAN("REG-EVT-089", USER_EVENT.getCode(), "REG_DOC_SCAN", "Doc: Click of Scan"),
REG_DOC_VIEW("REG-EVT-090", USER_EVENT.getCode(), "REG_DOC_VIEW", "Doc: View"),
REG_DOC_DELETE("REG-EVT-091", USER_EVENT.getCode(), "REG_DOC_DELETE", "Doc: Delete"),
Expand Down
2 changes: 2 additions & 0 deletions ...tration-services/src/main/java/io/mosip/registration/constants/RegistrationConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,8 @@ private RegistrationConstants() {
// flag for quality check with SDK
public static final String QUALITY_CHECK_WITH_SDK = "mosip.registration.quality_check_with_sdk";
public static final String UPDATE_SDK_QUALITY_SCORE = "mosip.registration.replace_sdk_quality_score";
public static final String SDK_QUALITY_TIMEOUT = "mosip.registration.sdk_quality_timeout_ms";
public static final long SDK_QUALITY_TIMEOUT_DEFAULT_MS = 5000L;

// Packet Sync
public static final String PACKET_SYNC = "packet_sync";
Expand Down
35 changes: 35 additions & 0 deletions ...n-services/src/main/java/io/mosip/registration/dto/biometric/QualityEvaluationResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.mosip.registration.dto.biometric;

/**
* Holds the outcome of a biometric quality evaluation, including the score,
* the source that produced it (SDK or SBI), and whether a fallback occurred.
*/
public class QualityEvaluationResult {

private final double score;
private final String source;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

source should be an enum or constant — raw String literals "SDK" / "SBI" are not type-safe.

BioServiceImpl hardcodes "SDK" and "SBI" at call sites. A new source type, a typo, or a refactor won't be caught at compile time. Introduce a small enum or reuse constants:

♻️ Proposed refactor 
+public enum QualitySource { SDK, SBI }
+
 public class QualityEvaluationResult {
 
     private final double score;
-    private final String source;
+    private final QualitySource source;
     private final boolean fallback;
 
-    public QualityEvaluationResult(double score, String source, boolean fallback) {
+    public QualityEvaluationResult(double score, QualitySource source, boolean fallback) {

Then in BioServiceImpl:

-return new QualityEvaluationResult(biometricsDto.getQualityScore(), "SBI", false);
+return new QualityEvaluationResult(biometricsDto.getQualityScore(), QualitySource.SBI, false);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/dto/biometric/QualityEvaluationResult.java`
at line 10, The field "source" in QualityEvaluationResult is a raw String and
should be replaced with a type-safe enum (e.g., SourceType { SDK, SBI }) or
shared constants; add a new enum SourceType (or constants class), change
QualityEvaluationResult.source from String to SourceType and update its
constructor, getter, equals/hashCode/serialization annotations if present, and
then update all call sites in BioServiceImpl to pass SourceType.SDK or
SourceType.SBI instead of the string literals "SDK"/"SBI"; ensure any JSON/XML
mappers are adjusted to serialize the enum as the expected string if needed.

private final boolean fallback;

public QualityEvaluationResult(double score, String source, boolean fallback) {
this.score = score;
this.source = source;
this.fallback = fallback;
}

public double getScore() {
return score;
}

public String getSource() {
return source;
}

public boolean isFallback() {
return fallback;
}

@Override
public String toString() {
return "QualityEvaluationResult{score=" + score + ", source='" + source + "', fallback=" + fallback + "}";
}
}
1 change: 1 addition & 0 deletions ...tration-services/src/main/java/io/mosip/registration/dto/packetmanager/BiometricsDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class BiometricsDto {
private boolean isCaptured;
private String subType;
private double sdkScore;
private String qualitySource;
private String payLoad;
private String signature;
private String specVersion;
Expand Down
5 changes: 5 additions & 0 deletions ...ervices/src/main/java/io/mosip/registration/exception/RegistrationExceptionConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ public enum RegistrationExceptionConstants {
REG_FINGERPRINT_SCANNING_ERROR(RegistrationConstants.USER_REG_FINGERPRINT_CAPTURE_EXP_CODE+"FCS-002", "Exception while scanning fingerprints of the individual"),
REG_BIOMETRIC_QUALITY_CHECK_ERROR("REG-BQC-001", "Exception while evaluating the biometrics quality with SDK"),
REG_BIOMETRIC_QUALITY_SCORE_RANGE_ERROR("REG-BQC-002", "Exception while evaluating the biometrics quality score"),
REG_SDK_TIMEOUT("REG-BQC-003", "Biometric quality evaluation timed out. Please re-capture biometrics."),
REG_SDK_NULL_RESPONSE("REG-BQC-004", "Invalid biometric quality score received from SDK."),
REG_SDK_RUNTIME_ERROR("REG-BQC-005", "Error occurred during SDK-based quality evaluation. Please retry capture."),
REG_SBI_SCORE_MISSING("REG-BQC-006", "Default biometric quality score unavailable."),
REG_NO_QUALITY_SOURCE("REG-BQC-007", "Biometric quality evaluation is not configured. Registration cannot proceed."),
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify REG_NO_QUALITY_SOURCE is never referenced outside its declaration
# Expect: only the declaration line in RegistrationExceptionConstants.java

rg -n --type=java "REG_NO_QUALITY_SOURCE"

Repository: mosip/registration-client

Length of output: 308

Remove the unused REG_NO_QUALITY_SOURCE constant.

The constant REG_NO_QUALITY_SOURCE (line 98) is defined but never thrown or referenced anywhere in the codebase. Either remove it or implement the exception handling path it was intended for.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/exception/RegistrationExceptionConstants.java`
at line 98, The REG_NO_QUALITY_SOURCE enum constant in
RegistrationExceptionConstants is unused; remove the
REG_NO_QUALITY_SOURCE("REG-BQC-007", "Biometric quality evaluation is not
configured. Registration cannot proceed.") entry from the
RegistrationExceptionConstants definition (adjust surrounding commas/formatting
so the enum remains valid) and run a build/tests to ensure no references remain;
if you intended to use it instead, implement the exception throw/path that
references REG_NO_QUALITY_SOURCE in the biometric quality evaluation code rather
than leaving the dead constant.

REG_OTP_VALIDATION("REG-OV-001","Erroe while validating the otp"),
REG_PACKET_DATE_PARSER_CODE(PACKET_CREATION_EXP_CODE + "TGE-001", "Exception while parsing the date to display in acknowledgement receipt"),
REG_PACKET_JSON_VALIDATOR_ERROR_CODE(PACKET_CREATION_EXP_CODE + "PCS-003", "Exception while validating ID json file"),
Expand Down
134 changes: 123 additions & 11 deletions ...tration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.io.InputStream;
import java.time.temporal.ValueRange;
import java.util.*;
import java.util.concurrent.*;

import io.mosip.registration.dto.schema.UiFieldDTO;
import io.mosip.registration.enums.Modality;
Expand All @@ -29,6 +30,7 @@
import io.mosip.registration.context.ApplicationContext;
import io.mosip.registration.context.SessionContext;
import io.mosip.registration.dto.RegistrationDTO;
import io.mosip.registration.dto.biometric.QualityEvaluationResult;
import io.mosip.registration.dto.packetmanager.BiometricsDto;
import io.mosip.registration.exception.RegBaseCheckedException;
import io.mosip.registration.exception.RegistrationExceptionConstants;
Expand Down Expand Up @@ -68,6 +70,8 @@ public class BioServiceImpl extends BaseService implements BioService {
@Autowired
private BIRBuilder birBuilder;

private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

sdkExecutor is never shut down — resource and thread leak in the Spring singleton.

Executors.newSingleThreadExecutor() creates a non-daemon thread. As a @Service singleton, BioServiceImpl lives for the application lifetime, but the executor is never torn down. On Spring context shutdown the thread stays alive, preventing clean JVM exit (critical for a JavaFX desktop app that expects clean process termination).

🛡️ Proposed fix — add `@PreDestroy` 
+import javax.annotation.PreDestroy;
 
 `@Service`
 public class BioServiceImpl extends BaseService implements BioService {
 
     private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
+
+    `@PreDestroy`
+    public void shutdownSdkExecutor() {
+        sdkExecutor.shutdownNow();
+    }

As per coding guidelines: "Flag use of ThreadLocal without guaranteed removal (memory leak risk)" — the same principle applies to any managed resource that holds threads.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
import javax.annotation.PreDestroy;
`@Service`
public class BioServiceImpl extends BaseService implements BioService {
private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
`@PreDestroy`
public void shutdownSdkExecutor() {
sdkExecutor.shutdownNow();
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java`
at line 73, BioServiceImpl creates a non-daemon ExecutorService (sdkExecutor)
that is never shut down, causing a thread/resource leak; add a lifecycle cleanup
method on the BioServiceImpl class annotated with `@PreDestroy` that shuts down
sdkExecutor (call shutdown(), optionally shutdownNow() on timeout, and
awaitTermination with a short timeout) and handle InterruptedException by
restoring the interrupt flag and logging; ensure the method references the
sdkExecutor field so the executor is properly terminated on Spring context
shutdown.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Single-thread executor can be permanently degraded by an unresponsive SDK.

future.cancel(true) on TimeoutException (line 236) only requests interruption of the running thread. If the SDK library does not check the interrupt flag, the thread remains blocked. Because sdkExecutor is a single-thread pool, every subsequent evaluateQuality() call enqueues behind the stuck task — all captures will time out serially until the JVM or SDK recovers. Under a misbehaving SDK this becomes a permanent throughput failure.

Consider replacing with a cached thread pool or a per-call thread, so each invocation gets a fresh thread and a stalled SDK call only affects its own biometric, not all future captures:

-private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
+// Each evaluation gets its own thread; stalled threads cannot block future evaluations.
+private final ExecutorService sdkExecutor = Executors.newCachedThreadPool();

Note: With a cached pool, a consistently misbehaving SDK would keep spawning threads. Add a bounded semaphore or thread factory with a sensible cap if needed for a production hardening pass.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private final ExecutorService sdkExecutor = Executors.newSingleThreadExecutor();
// Each evaluation gets its own thread; stalled threads cannot block future evaluations.
private final ExecutorService sdkExecutor = Executors.newCachedThreadPool();
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java`
at line 73, The current single-thread ExecutorService sdkExecutor (created via
Executors.newSingleThreadExecutor()) can be permanently blocked by an
unresponsive SDK call from evaluateQuality(); change the executor strategy so a
stuck SDK call won’t block all future evaluateQuality() invocations — e.g.,
replace sdkExecutor with a pooled or per-call executor such as
Executors.newCachedThreadPool() or create a fresh single-thread executor per
evaluateQuality() invocation, and limit growth using a bounded ThreadFactory,
Semaphore, or capped thread pool to prevent unbounded thread creation; update
usages that submit/ cancel tasks to use the new executor and ensure proper
shutdown of per-call executors if you choose that approach.


/**
* Gets the registration DTO from session.
*
Expand Down Expand Up @@ -97,16 +101,11 @@ public List<BiometricsDto> captureModality(MDMRequestDto mdmRequestDto) throws R
throw new RegBaseCheckedException(RegistrationExceptionConstants.REG_BIOMETRIC_QUALITY_SCORE_RANGE_ERROR.getErrorCode(),
RegistrationExceptionConstants.REG_BIOMETRIC_QUALITY_SCORE_RANGE_ERROR.getErrorMessage());

if (RegistrationConstants.ENABLE.equalsIgnoreCase((String) ApplicationContext.map()
.getOrDefault(RegistrationConstants.QUALITY_CHECK_WITH_SDK, RegistrationConstants.DISABLE))) {
try {
biometricsDto.setSdkScore(getSDKScore(biometricsDto));
} catch (BiometricException e) {
LOGGER.error("Unable to fetch SDK Score ", e);
throw new RegBaseCheckedException(RegistrationExceptionConstants.REG_BIOMETRIC_QUALITY_CHECK_ERROR.getErrorCode(),
RegistrationExceptionConstants.REG_BIOMETRIC_QUALITY_CHECK_ERROR.getErrorMessage());
}
}
QualityEvaluationResult result = evaluateQuality(biometricsDto);
biometricsDto.setSdkScore(result.getScore());
biometricsDto.setQualitySource(result.getSource());
LOGGER.info("Quality evaluation complete: score={}, source={}, fallback={}",
result.getScore(), result.getSource(), result.isFallback());
list.add(biometricsDto);
}
} catch (RegBaseCheckedException e) {
Expand Down Expand Up @@ -161,6 +160,111 @@ public InputStream getStream(MdmBioDevice mdmBioDevice, String modality) throws
RegistrationExceptionConstants.MDS_BIODEVICE_NOT_FOUND.getErrorMessage());
}

/**
* Orchestrates biometric quality evaluation across SDK and SBI sources.
*
* Decision rules:
* - SDK disabled in config: use SBI score (AS-03)
* - SDK enabled but provider unbound: fall back to SBI score (AS-02)
* - SDK enabled, provider bound, call fails (timeout/null/exception): block, no fallback
*/
private QualityEvaluationResult evaluateQuality(BiometricsDto biometricsDto) throws RegBaseCheckedException {
boolean sdkEnabled = RegistrationConstants.ENABLE.equalsIgnoreCase(
(String) ApplicationContext.map().getOrDefault(
RegistrationConstants.QUALITY_CHECK_WITH_SDK, RegistrationConstants.DISABLE));

if (!sdkEnabled) {
LOGGER.info("SDK quality evaluation disabled, using SBI score for {}", biometricsDto.getBioAttribute());
return new QualityEvaluationResult(biometricsDto.getQualityScore(), "SBI", false);
}

BiometricType biometricType = BiometricType
.fromValue(io.mosip.commons.packet.constants.Biometric
.getSingleTypeByAttribute(biometricsDto.getBioAttribute()).name());

// Check provider availability before committing to SDK path
Object provider = null;
try {
provider = bioAPIFactory.getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK);
} catch (Exception e) {
LOGGER.warn("SDK provider not available for {}, falling back to SBI", biometricType);
}

if (provider == null) {
LOGGER.info("SDK provider unbound, falling back to SBI score for {}", biometricsDto.getBioAttribute());
double sbiScore = biometricsDto.getQualityScore();
if (sbiScore <= 0) {
LOGGER.error("SBI score also unavailable for {}", biometricsDto.getBioAttribute());
throw new RegBaseCheckedException(
RegistrationExceptionConstants.REG_SBI_SCORE_MISSING.getErrorCode(),
RegistrationExceptionConstants.REG_SBI_SCORE_MISSING.getErrorMessage());
}
return new QualityEvaluationResult(sbiScore, "SBI", true);
}

// SDK provider is bound - call with timeout. Any failure here blocks registration.
long timeoutMs = getSdkTimeoutMs();
BIR bir = birBuilder.buildBir(biometricsDto, ProcessedLevelType.RAW);
BIR[] birList = new BIR[]{bir};

Future<Double> future = sdkExecutor.submit(() -> {
try {
Map<BiometricType, Float> scoreMap = bioAPIFactory
.getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK)
.getModalityQuality(birList, null);
return (scoreMap != null && scoreMap.get(biometricType) != null)
? scoreMap.get(biometricType).doubleValue()
: null;
} catch (Exception e) {
LOGGER.error("SDK provider threw during quality check", e);
throw new RuntimeException(e);
}
});
Comment on lines +186 to +222
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

TOCTOU: getBioProvider() is called twice, but the obtained provider reference is discarded.

At line 188 the provider is fetched and stored as Object solely to check for null. The actual lambda (lines 212–213) calls getBioProvider() a second time. This means:

  1. The availability check and the actual invocation are non-atomic (TOCTOU).
  2. The first getBioProvider() result is always discarded — the allocation is wasted.
  3. If the provider becomes unbound between the two calls, the exception is caught inside the lambda and re-wrapped as REG_SDK_RUNTIME_ERROR rather than triggering the SBI fallback path.

Use the acquired reference in the lambda to eliminate both problems:

♻️ Proposed fix 
-        Object provider = null;
+        // Use var (Java 10+) or the concrete return type of getBioProvider
+        var provider = (Object) null;
         try {
-            provider = bioAPIFactory.getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK);
+            provider = bioAPIFactory.getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK);
         } catch (Exception e) {
             LOGGER.warn("SDK provider not available for {}, falling back to SBI", biometricType);
         }
 
         if (provider == null) { ... }
 
+        final var boundProvider = bioAPIFactory.getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK);
         Future<Double> future = sdkExecutor.submit(() -> {
             try {
-                Map<BiometricType, Float> scoreMap = bioAPIFactory
-                        .getBioProvider(biometricType, BiometricFunction.QUALITY_CHECK)
-                        .getModalityQuality(birList, null);
+                Map<BiometricType, Float> scoreMap = boundProvider.getModalityQuality(birList, null);

Or, simpler — capture the result of the first getBioProvider() with the proper interface type and use finalProvider in the lambda, skipping the redundant second call entirely.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java`
around lines 186 - 222, The code calls bioAPIFactory.getBioProvider(...) twice
(stored in a raw Object first and called again inside the lambda), causing a
TOCTOU and discarding the first result; fix by capturing the provider once into
a properly typed variable (replace Object provider with the concrete provider
interface returned by getBioProvider, e.g., final IYourProviderType
finalProvider = bioAPIFactory.getBioProvider(biometricType,
BiometricFunction.QUALITY_CHECK)), keep the existing null-check and SBI
fallback, and then use finalProvider inside the Future lambda instead of calling
bioAPIFactory.getBioProvider(...) again so the same provider instance is used
and the redundant allocation/call is removed.


try {
Double score = future.get(timeoutMs, TimeUnit.MILLISECONDS);
if (score == null) {
LOGGER.error("SDK returned null quality score for {}", biometricsDto.getBioAttribute());
throw new RegBaseCheckedException(
RegistrationExceptionConstants.REG_SDK_NULL_RESPONSE.getErrorCode(),
RegistrationExceptionConstants.REG_SDK_NULL_RESPONSE.getErrorMessage());
}
LOGGER.info("SDK quality score {} received for {}", score, biometricsDto.getBioAttribute());
return new QualityEvaluationResult(score, "SDK", false);

} catch (TimeoutException e) {
future.cancel(true);
LOGGER.error("SDK quality evaluation timed out after {}ms for {}", timeoutMs, biometricsDto.getBioAttribute());
throw new RegBaseCheckedException(
RegistrationExceptionConstants.REG_SDK_TIMEOUT.getErrorCode(),
RegistrationExceptionConstants.REG_SDK_TIMEOUT.getErrorMessage());

} catch (ExecutionException e) {
LOGGER.error("SDK runtime error during quality evaluation for {}", biometricsDto.getBioAttribute(), e);
throw new RegBaseCheckedException(
RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorCode(),
RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorMessage());

} catch (RegBaseCheckedException e) {
throw e;

} catch (Exception e) {
LOGGER.error("Unexpected error during SDK quality evaluation", e);
throw new RegBaseCheckedException(
RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorCode(),
RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorMessage());
Comment on lines +235 to +255
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

InterruptedException is swallowed without restoring the interrupt flag.

future.get(timeoutMs, MILLISECONDS) can throw InterruptedException. It is caught by the final catch (Exception e) block (line 251) and silently converted to REG_SDK_RUNTIME_ERROR. This clears the calling thread's interrupt status, making the interruption invisible to any outer code that checks Thread.isInterrupted().

🛡️ Proposed fix — add an explicit `InterruptedException` catch 
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt(); // restore interrupt status
+            future.cancel(true);
+            throw new RegBaseCheckedException(
+                    RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorCode(),
+                    RegistrationExceptionConstants.REG_SDK_RUNTIME_ERROR.getErrorMessage());
         } catch (TimeoutException e) {
             future.cancel(true);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java`
around lines 235 - 255, The catch-all Exception block in BioServiceImpl is
swallowing InterruptedException from future.get(timeoutMs, MILLISECONDS) and
clearing the thread interrupt status; add an explicit catch
(InterruptedException e) immediately before the general Exception catch, restore
the interrupt flag with Thread.currentThread().interrupt(), log context (e.g.,
"Thread interrupted during SDK quality evaluation for {}" with
biometricsDto.getBioAttribute()), and rethrow or wrap as a
RegBaseCheckedException consistent with other handlers so interruption is not
lost.

}
}

private long getSdkTimeoutMs() {
String configured = getGlobalConfigValueOf(RegistrationConstants.SDK_QUALITY_TIMEOUT);
try {
return configured != null ? Long.parseLong(configured) : RegistrationConstants.SDK_QUALITY_TIMEOUT_DEFAULT_MS;
} catch (NumberFormatException e) {
return RegistrationConstants.SDK_QUALITY_TIMEOUT_DEFAULT_MS;
}
}
Comment on lines +163 to +266
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify whether any of the new SDK audit events are fired elsewhere,
# and confirm the audit helper method name on BaseService.

# Check for any usage of the new audit event constants
rg -n --type=java "SDK_QUALITY_SUCCESS|SDK_QUALITY_FAILED|SDK_FALLBACK_TO_SBI|SDK_QUALITY_DISABLED|NO_VALID_QUALITY_SCORE"

# Check what audit helper methods BaseService exposes
ast-grep --pattern 'class BaseService {
  $$$
  public $_ audit($$$) {
    $$$
  }
  $$$
}'

Repository: mosip/registration-client

Length of output: 1283

🏁 Script executed:

# Find BioServiceImpl class and its parent
rg -n --type=java "class BioServiceImpl" -A 3

# Search for audit() method calls in service classes to understand the pattern
rg -n --type=java "audit\(" | head -30

# Check if BioServiceImpl has access to auditFactory or similar
rg -n --type=java "auditFactory|AuditFactory" registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java

# Search for existing audit calls in BaseService or other bio service methods
fd BioServiceImpl.java --exec cat -n {} \;

Repository: mosip/registration-client

Length of output: 27177

🏁 Script executed:

# Find BaseService and check for auditFactory
fd BaseService.java --exec head -100 {} \;

# Also search for how auditFactory is provided
rg -n --type=java "protected.*auditFactory|private.*auditFactory|@Autowired.*auditFactory" -A 2 | head -30

Repository: mosip/registration-client

Length of output: 7770

🏁 Script executed:

# Search entire BaseService for auditFactory
rg -n "auditFactory" registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

# Or find the file and check its full length
wc -l registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

Repository: mosip/registration-client

Length of output: 169

🏁 Script executed:

# Search for auditFactory declaration in BaseService
grep -n "auditFactory" registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

Repository: mosip/registration-client

Length of output: 51

🏁 Script executed:

# Search for how audit is done across service classes - look for audit pattern
rg -n "audit\(" registration/registration-services/src/main/java/io/mosip/registration/service/bio/ -B 2 -A 2

# Find where AuditEvent is imported and used
rg -n "import.*AuditEvent" registration/registration-services/src/main/java/io/mosip/registration/service/

# Check a service that uses auditFactory to see how it's imported/injected
rg -n "auditFactory" registration/registration-services/src/main/java/io/mosip/registration/service/login/impl/LoginServiceImpl.java -B 5 -A 2 | head -40

Repository: mosip/registration-client

Length of output: 2693

🏁 Script executed:

# Check if BioServiceImpl imports AuditEvent
grep -n "import.*AuditEvent\|import.*AuditManagerService\|import.*Components" registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java

# Find BaseService's auditFactory field
sed -n '70,120p' registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

Repository: mosip/registration-client

Length of output: 1815

🏁 Script executed:

# Search for auditFactory field in BaseService
sed -n '1,200p' registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java | grep -n "auditFactory" -A 2 -B 2

# Also check around line 100-150
sed -n '100,150p' registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

Repository: mosip/registration-client

Length of output: 1455

🏁 Script executed:

# Continue searching for auditFactory in BaseService - it might be further down
grep -n "auditFactory\|AuditManagerService\|AuditEvent" registration/registration-services/src/main/java/io/mosip/registration/service/BaseService.java

# If not found, check if child classes need to define it themselves
rg -n "@Autowired.*auditFactory" registration/registration-services/src/main/java/io/mosip/registration/service/ -B 1

Repository: mosip/registration-client

Length of output: 51

🏁 Script executed:

# Confirm BioServiceImpl has no auditFactory field or AuditEvent imports
head -50 registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java | grep -i "audit"

# Check if evaluateQuality is called from multiple places
rg -n "evaluateQuality" registration/registration-services/src/main/java/io/mosip/registration/service/bio/

Repository: mosip/registration-client

Length of output: 473

Audit events defined but never emitted in quality evaluation

evaluateQuality() defines decision rules for SDK quality evaluation but uses only LOGGER calls. None of the five audit events (SDK_QUALITY_SUCCESS, SDK_QUALITY_FAILED, SDK_FALLBACK_TO_SBI, SDK_QUALITY_DISABLED, NO_VALID_QUALITY_SCORE) are fired in any code path. In MOSIP, biometric quality evaluation requires an auditable trail for compliance; the current implementation silently succeeds or fails with no audit record.

Add @Autowired for AuditManagerService and emit the corresponding audit event at each decision branch:

  • SDK disabled: auditFactory.audit(AuditEvent.SDK_QUALITY_DISABLED, ...)
  • Provider unbound fallback: auditFactory.audit(AuditEvent.SDK_FALLBACK_TO_SBI, ...)
  • Success: auditFactory.audit(AuditEvent.SDK_QUALITY_SUCCESS, ...)
  • Timeout/null/error paths: auditFactory.audit(AuditEvent.SDK_QUALITY_FAILED, ...)

See how other service classes (e.g., LoginServiceImpl, PacketHandlerServiceImpl) emit audit events for reference on the correct pattern.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@registration/registration-services/src/main/java/io/mosip/registration/service/bio/impl/BioServiceImpl.java`
around lines 163 - 266, The evaluateQuality(...) method currently logs decisions
but never emits audit events; add an `@Autowired` AuditManagerService (or existing
auditFactory) field to the class and call auditFactory.audit(...) with the
appropriate AuditEvent at each branch: on SDK disabled emit
AuditEvent.SDK_QUALITY_DISABLED in the early !sdkEnabled branch; when provider
is null emit AuditEvent.SDK_FALLBACK_TO_SBI before falling back to SBI; on
successful SDK score return emit AuditEvent.SDK_QUALITY_SUCCESS (include score
and biometric attribute); and in timeout/null/exception/ExecutionException paths
emit AuditEvent.SDK_QUALITY_FAILED (include error details and biometric
attribute); follow the same audit payload pattern used in
LoginServiceImpl/PacketHandlerServiceImpl to construct event data.


@Override
public double getSDKScore(BiometricsDto biometricsDto) throws BiometricException {
BiometricType biometricType = BiometricType
Expand Down Expand Up @@ -196,7 +300,15 @@ public Map<String, Boolean> getCapturedBiometrics(@NonNull UiFieldDTO fieldDto,
capturedContext.put(attribute, true);
continue;
}
quality = quality + biometricsDto.getQualityScore();
double effectiveScore = (biometricsDto.getSdkScore() > 0)
? biometricsDto.getSdkScore()
: biometricsDto.getQualityScore();
if (effectiveScore <= 0) {
LOGGER.warn("No valid quality score available for attribute {}", attribute);
capturedContext.put(attribute, false);
continue;
}
quality = quality + effectiveScore;
capturedAttributes.add(attribute);
}
//if some attributes are captured, determine capture status based on threshold check
Expand Down