Skip to content

[DMP 2026] Enhancement: Track Supervisor ID in Registration Packet Processing Closes #2322#2344

Open
Adithkp03 wants to merge 2 commits into
mosip:masterfrom
Adithkp03:feat/supervisor-id-sync-v3
Open

[DMP 2026] Enhancement: Track Supervisor ID in Registration Packet Processing Closes #2322#2344
Adithkp03 wants to merge 2 commits into
mosip:masterfrom
Adithkp03:feat/supervisor-id-sync-v3

Conversation

@Adithkp03

@Adithkp03 Adithkp03 commented May 12, 2026

Copy link
Copy Markdown

Summary
Adds Sync V3 support for supervisor traceability in the Registration Processor. Previously, packet approval and rejection captured status, comments, and timestamps — but not the identity of the supervisor or admin who performed the action. This PR introduces a new /syncV3 endpoint, persists supervisorId and source to the database, and enforces validation logic in the supervisor processing stage to support reporting and auditability.

Demo

Architecture diagram
arch diagram

Walkthrough video
▶ Watch demo
Covers: problem statement · architecture · code walkthrough · DB migration · test run · reporting query


Changes
New endpoint

POST /registrationprocessor/v1/registrationstatus/syncV3 added alongside the existing /syncV2 (unchanged)

DTO

SyncRegistrationDto — added supervisorId and source fields

Service

SyncRegistrationServiceImpl.syncV3() — new method implementing:

SUPERVISOR_UPLOAD: requires supervisorId, cross-checks against packet optionalValues metadata if present
ADMIN_UPLOAD: requires supervisorId (admin login ID), skips downstream supervisor validation
Legacy/null: no source and no supervisorId — passes through for backward compatibility with V2 clients

Entity

SyncRegistrationEntity — added supervisorId field mapped to new supervisor_id column

Processor

SupervisorValidationProcessor — refactored to branch on source:

ADMIN_UPLOAD → skip validation, log warning
Legacy packet (no source, no supervisorId anywhere) → pass through
Otherwise → match stored supervisorId against packet metadata, then run existing supervisor validator

Error constants

PlatformErrorMessages — added RPR_RGS_SUPERVISOR_ID_REQUIRED, RPR_RGS_SUPERVISOR_ID_MISMATCH, RPR_RGS_INVALID_SYNCV3_REQUEST, RPR_RGS_SUPERVISOR_DETAILS_SAVE_FAILED

Database

registration_list — new nullable supervisor_id character varying column
Partial index idx_rgstrnlst_supervisor_status on (supervisor_id, source, client_status_code) WHERE supervisor_id IS NOT NULL
Upgrade script: db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_upgrade.sql
Rollback script: db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_rollback.sql

Backward Compatibility

/syncV2 is completely unchanged
supervisor_id is nullable — all existing records and legacy packets are unaffected
V2 clients sending no source and no supervisorId via /syncV3 are handled as a legacy pass-through

Validation Logic
IF source = ADMIN_UPLOAD
→ supervisorId required (admin login ID)
→ skip downstream supervisor validation
→ mark as admin-uploaded

IF source = SUPERVISOR_UPLOAD
→ supervisorId required
→ if optionalValues contains supervisorId label → must match
→ run existing supervisor validator at processing stage

IF no source AND no supervisorId (legacy)
→ allow through, null stored

Tests
SyncRegistrationServiceTest

testGetSyncRegistrationStatusV3SuccessPersistsSupervisor — ArgumentCaptor proves supervisorId and source reach the DAO
testGetSyncRegistrationStatusV3MissingSupervisorFailure — missing supervisorId blocks DB write
testSyncV3SupervisorMetadataMismatchFails — metadata mismatch returns error, no DB write
testSyncV3AdminUploadPersistsAdminSupervisorId — admin login ID stored as supervisorId
testSyncV3AdminUploadMissingSupervisorAndNoPrincipalFails — admin upload without supervisorId fails
testSyncV3SupervisorMetadataMatchSucceeds — matching metadata proceeds and persists correctly
testSyncV2BackwardCompatibleNoSourceNoSupervisor — null stored, status SUCCESS, V2 behaviour unchanged

Test Results

SyncRegistrationServiceTest
image

SupervisorValidatorProcessorTest
image

RegistrationStatusAndSyncControllerTest
image


SupervisorValidatorProcessorTest

adminUploadSkipsSupervisorValidationTest — validator never called for admin packets
supervisorIdMismatchTest — mismatched IDs set isValid=false
legacyPacketWithoutSupervisorIdTest — legacy packets pass through, validator never called

RegistrationStatusAndSyncControllerTest

testSyncV3Controller — /syncV3 endpoint returns HTTP 200

Reporting
Once deployed, supervisor vs packet-status analytics are available directly:
sqlSELECT supervisor_id, source, client_status_code, COUNT(*) AS packet_count
FROM regprc.registration_list
WHERE supervisor_id IS NOT NULL
GROUP BY supervisor_id, source, client_status_code
ORDER BY supervisor_id, source, client_status_code;

Out of Scope (follow-up)

Registration Client wiring to include supervisorId in the encrypted sync payload (separate module, separate PR)
Admin Portal integration

Documentation
docs/sync-v3-supervisor-traceability.md — covers the problem, API contract, DB changes, reporting query, and error handling.

Mid-point Milestone Checklist

Sync V3 API updated to include supervisorId
Registration client sends supervisorId during sync
Basic persistence of supervisorId in DB
Initial validation logic implemented

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Sync V3 endpoint enabling enhanced registration synchronization with supervisor and administrator upload source tracking.
    • Supervisor validation now intelligently handles administrative versus supervisor uploads with appropriate conditional logic.
    • Registrations now capture supervisor ID and upload source metadata for improved traceability.
  • Documentation

    • Added comprehensive documentation for Sync V3 supervisor traceability feature and validation workflows.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Walkthrough

This PR implements Sync V3 supervisor traceability for the Registration Processor. The change adds supervisor ID and source metadata persistence to track whether registrations come from admin or supervisor uploads, enabling conditional supervisor validation bypass for admin uploads and supervisorId metadata cross-checks for supervisor uploads.

Changes

Sync V3 Supervisor Traceability

Layer / File(s) Summary
Database Schema & Persistence Layer
db_scripts/mosip_regprc/ddl/regprc-registration_list.sql, db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_*.sql, registration-processor/.../entity/SyncRegistrationEntity.java, registration-processor/.../dto/SyncRegistrationDto.java
Schema adds supervisor_id column with partial index on (supervisor_id, source, client_status_code). Entity and DTO layers map supervisorId and source fields for persistence.
Service Interface & Error Codes
registration-processor/.../service/SyncRegistrationService.java, registration-processor/.../exception/util/PlatformErrorMessages.java
Service interface defines new syncV3(...) method. Four error codes added for supervisor ID required/mismatch, invalid sync V3 request, and persistence failure.
Sync V3 Core Service Implementation
registration-processor/.../service/impl/SyncRegistrationServiceImpl.java
Core syncV3 method iterates packets, validates upload source (ADMIN_UPLOAD, SUPERVISOR_UPLOAD, legacy), resolves supervisorId from authenticated principal for admin uploads, cross-checks metadata supervisorId for supervisor uploads, persists supervisor metadata, and returns V2-compatible response DTOs with per-packet error reporting.
Controller /syncV3 Endpoint & Response Building
registration-processor/.../api/controller/RegistrationSyncController.java, registration-processor/.../test/.../RegistrationStatusAndSyncControllerTest.java
New /syncV3 endpoint decrypts/validates request and calls syncRegistrationService.syncV3(...). Response builder extended to map SyncResponseFailureV2Dto entries with registrationId and status. Controller test added for endpoint integration.
Supervisor Validator Processor Integration
registration-processor/.../supervisorvalidator/SupervisorValidationProcessor.java, registration-processor/.../test/.../SupervisorValidatorProcessorTest.java
Processor refactored to fetch sync data and branch validation: admin uploads and legacy packets skip supervisor validation; others require supervisorId presence, validate supervisorId matches sync record, then execute supervisor validation. Tests cover admin upload bypass, supervisorId mismatch rejection, and legacy packet handling.
Comprehensive Service and Processor Tests
registration-processor/.../service/SyncRegistrationServiceTest.java
Service tests cover syncV3 supervisor persistence, missing supervisor failure, backward compatibility without source/supervisorId, admin supervisorId resolution, principal-based supervisorId fallback, and supervisor metadata match/mismatch validation using optionalValues.
Sync V3 Requirements Documentation
docs/sync-v3-supervisor-traceability.md
Documents request fields, persistence model, validation rules by upload source, encrypted metadata contract, reporting example, and error handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hops through the database rows,
Supervisor IDs now flow,
Admin uploads skip the check,
Metadata matches—all correct!
Traceability's here to stay,
Sync V3 leads the way!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.13% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding Sync V3 support for supervisor ID tracking in the registration packet processing system, which is the primary focus of this pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with 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.

Inline comments:
In `@db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_rollback.sql`:
- Around line 1-4: Add a preamble comment to the rollback script advising
operators to back up/export data before dropping the supervisor_id column and
removing the index; specifically mention that
regprc.registration_list.supervisor_id will be permanently lost and recommend
taking a table dump or exporting supervisory data. Place the comment near the
top of the script above the DROP INDEX/DROP COLUMN statements (referencing DROP
INDEX IF EXISTS regprc.idx_rgstrnlst_supervisor_status and DROP COLUMN IF EXISTS
supervisor_id) so anyone running the rollback sees the backup guidance first.
Ensure the comment is concise and actionable (e.g., backup commands or pointers
to backup procedure).

In
`@registration-processor/init/registration-processor-registration-status-service/src/main/java/io/mosip/registration/processor/status/api/controller/RegistrationSyncController.java`:
- Around line 176-221: The controller method syncRegistrationController3
currently only catches JsonProcessingException; add broader handling around
service-layer failures by catching service/unchecked exceptions thrown by
syncRegistrationService.syncV3 and related calls (e.g., catch
RegStatusAppException and Exception around decryptAndGetSyncRequest(),
validator.validate(), and syncRegistrationService.syncV3) and convert or rethrow
them as RegStatusAppException with a clear PlatformErrorMessages code and
include the original exception for context; ensure you also log the error before
throwing so failures in syncV3 are recorded and the response still goes through
the standard error flow.

In
`@registration-processor/pre-processor/registration-processor-supervisor-validator-stage/src/main/java/io/mosip/registration/processor/stages/supervisorvalidator/SupervisorValidationProcessor.java`:
- Around line 211-222: The getSyncRegistrationEntity method silently picks the
first element when syncRegistrationService.findByRegistrationId(...) returns
multiple entries; update getSyncRegistrationEntity to detect when the returned
List has size > 1 and handle it defensively: either log a warning via the
component logger including registrationId and workflowInstanceId, or choose the
most appropriate entity (e.g., the latest by updated/created timestamp) before
returning; reference syncRegistrationService.findByRegistrationId and
syncRegistrationEntities.get(0) to locate the selection point and add the
logging/selection logic there.

In
`@registration-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/service/impl/SyncRegistrationServiceImpl.java`:
- Around line 566-591: The reflection-based resolveAuthenticatedPrincipalName in
SyncRegistrationServiceImpl is fragile; replace it by creating an
AuthenticationFacade component that exposes getAuthenticatedUsername() which
reads SecurityContextHolder.getContext().getAuthentication(), checks for
null/authenticated and filters "anonymousUser", then inject that facade into
SyncRegistrationServiceImpl (autowire required=false) and rewrite
resolveAuthenticatedPrincipalName to return authenticationFacade != null ?
authenticationFacade.getAuthenticatedUsername() : null; ensure the facade class
name and method (AuthenticationFacade.getAuthenticatedUsername) and the existing
method resolveAuthenticatedPrincipalName are used so callers remain unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9031f25c-f1ee-44ae-b0c9-906af00cedd8

📥 Commits

Reviewing files that changed from the base of the PR and between 290b418 and 6d8b1f9.

📒 Files selected for processing (14)
  • db_scripts/mosip_regprc/ddl/regprc-registration_list.sql
  • db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_rollback.sql
  • db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_upgrade.sql
  • docs/sync-v3-supervisor-traceability.md
  • registration-processor/init/registration-processor-registration-status-service/src/main/java/io/mosip/registration/processor/status/api/controller/RegistrationSyncController.java
  • registration-processor/init/registration-processor-registration-status-service/src/test/java/io/mosip/registration/processor/status/api/controller/RegistrationStatusAndSyncControllerTest.java
  • registration-processor/pre-processor/registration-processor-supervisor-validator-stage/src/main/java/io/mosip/registration/processor/stages/supervisorvalidator/SupervisorValidationProcessor.java
  • registration-processor/pre-processor/registration-processor-supervisor-validator-stage/src/test/java/io/mosip/registration/processor/app/SupervisorValidatorProcessorTest.java
  • registration-processor/registration-processor-core/src/main/java/io/mosip/registration/processor/core/exception/util/PlatformErrorMessages.java
  • registration-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/dto/SyncRegistrationDto.java
  • registration-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/entity/SyncRegistrationEntity.java
  • registration-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/service/SyncRegistrationService.java
  • registration-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/service/impl/SyncRegistrationServiceImpl.java
  • registration-processor/registration-processor-registration-status-service-impl/src/test/java/io/mosip/registration/processor/status/service/SyncRegistrationServiceTest.java

Comment on lines +1 to +4
DROP INDEX IF EXISTS regprc.idx_rgstrnlst_supervisor_status;

ALTER TABLE regprc.registration_list
DROP COLUMN IF EXISTS supervisor_id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider documenting data backup recommendation.

The rollback script drops the supervisor_id column, which will permanently delete any supervisor traceability data if the rollback is executed. While the column is nullable and the operation is safe from a schema perspective, consider adding a comment recommending data backup or export before running the rollback.

📝 Suggested documentation addition
+-- WARNING: This rollback will permanently delete supervisor_id data.
+-- Consider exporting supervisor traceability reports before running this script.
+
 DROP INDEX IF EXISTS regprc.idx_rgstrnlst_supervisor_status;

 ALTER TABLE regprc.registration_list
 	DROP COLUMN IF EXISTS supervisor_id;
📝 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
DROP INDEX IF EXISTS regprc.idx_rgstrnlst_supervisor_status;
ALTER TABLE regprc.registration_list
DROP COLUMN IF EXISTS supervisor_id;
-- WARNING: This rollback will permanently delete supervisor_id data.
-- Consider exporting supervisor traceability reports before running this script.
DROP INDEX IF EXISTS regprc.idx_rgstrnlst_supervisor_status;
ALTER TABLE regprc.registration_list
DROP COLUMN IF EXISTS supervisor_id;
🤖 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 `@db_upgrade_scripts/mosip_regprc/sql/1.3.0_to_1.3.1_rollback.sql` around lines
1 - 4, Add a preamble comment to the rollback script advising operators to back
up/export data before dropping the supervisor_id column and removing the index;
specifically mention that regprc.registration_list.supervisor_id will be
permanently lost and recommend taking a table dump or exporting supervisory
data. Place the comment near the top of the script above the DROP INDEX/DROP
COLUMN statements (referencing DROP INDEX IF EXISTS
regprc.idx_rgstrnlst_supervisor_status and DROP COLUMN IF EXISTS supervisor_id)
so anyone running the rollback sees the backup guidance first. Ensure the
comment is concise and actionable (e.g., backup commands or pointers to backup
procedure).

Comment on lines +176 to 221
/**
* Sync registration ids with supervisor/admin traceability fields.
*
* @param syncRegistrationList
* the sync registration list
* @return the response entity
* @throws RegStatusAppException
*/
@PreAuthorize("hasAnyRole(@authorizedRoles.getPostsyncv2())")
@PostMapping(path = "/syncV3", consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
@Operation(summary = "Get the synchronizing registration entity with supervisor traceability", description = "Get the synchronizing registration entity with supervisor traceability", tags = { "Registration Status" })
@ApiResponses(value = {
@ApiResponse(responseCode = "200", description = "Synchronizing Registration Entity successfully fetched",
content = @Content(schema = @Schema(implementation = RegistrationStatusCode.class))),
@ApiResponse(responseCode = "201", description = "Created" ,content = @Content(schema = @Schema(hidden = true))) ,
@ApiResponse(responseCode = "401", description = "Unauthorized" ,content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "403", description = "Forbidden" ,content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "404", description = "Not Found" ,content = @Content(schema = @Schema(hidden = true)))})
public ResponseEntity<Object> syncRegistrationController3(
@RequestHeader(name = "Center-Machine-RefId", required = true) String referenceId,
@RequestHeader(name = "timestamp", required = true) String timeStamp,
@RequestBody(required = true) Object encryptedSyncMetaInfo) throws RegStatusAppException {
try {
List<SyncResponseDto> syncResponseList = new ArrayList<>();
RegistrationSyncRequestDTO registrationSyncRequestDTO = syncRegistrationService
.decryptAndGetSyncRequest(encryptedSyncMetaInfo, referenceId, timeStamp, syncResponseList);

if (registrationSyncRequestDTO != null && validator.validate(registrationSyncRequestDTO,
env.getProperty(REG_SYNC_SERVICE_ID), syncResponseList)) {
syncResponseList = syncRegistrationService.syncV3(registrationSyncRequestDTO.getRequest(), referenceId, timeStamp);
}
RegSyncResponseDTO responseDto = buildRegistrationSyncResponse(syncResponseList);
String res = objectMapper.writeValueAsString(responseDto);
if (isEnabled) {
HttpHeaders headers = new HttpHeaders();
headers.add(RESPONSE_SIGNATURE,
digitalSignatureUtility.getDigitalSignature(res));
return ResponseEntity.ok().headers(headers).body(res);
}

return ResponseEntity.ok().body(res);

} catch (JsonProcessingException e) {
throw new RegStatusAppException(PlatformErrorMessages.RPR_RGS_DATA_VALIDATION_FAILED, e);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Consider broader exception handling for service layer failures.

The syncRegistrationController3 method only catches JsonProcessingException. While this is consistent with the existing syncV2 pattern, uncaught exceptions from the service layer (e.g., database errors, validation failures) will be handled by the global exception handler. For better error context and logging, consider explicitly handling service-layer exceptions here or ensure the global handler provides sufficient detail for Sync V3 failures.

🤖 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-processor/init/registration-processor-registration-status-service/src/main/java/io/mosip/registration/processor/status/api/controller/RegistrationSyncController.java`
around lines 176 - 221, The controller method syncRegistrationController3
currently only catches JsonProcessingException; add broader handling around
service-layer failures by catching service/unchecked exceptions thrown by
syncRegistrationService.syncV3 and related calls (e.g., catch
RegStatusAppException and Exception around decryptAndGetSyncRequest(),
validator.validate(), and syncRegistrationService.syncV3) and convert or rethrow
them as RegStatusAppException with a clear PlatformErrorMessages code and
include the original exception for context; ensure you also log the error before
throwing so failures in syncV3 are recorded and the response still goes through
the standard error flow.

Comment on lines +211 to +222
private SyncRegistrationEntity getSyncRegistrationEntity(String registrationId, String workflowInstanceId) {
SyncRegistrationEntity syncRegistrationEntity = null;
if (workflowInstanceId != null) {
syncRegistrationEntity = syncRegistrationService.findByWorkflowInstanceId(workflowInstanceId);
}
if (syncRegistrationEntity == null) {
List<SyncRegistrationEntity> syncRegistrationEntities = syncRegistrationService.findByRegistrationId(registrationId);
syncRegistrationEntity = syncRegistrationEntities != null && !syncRegistrationEntities.isEmpty()
? syncRegistrationEntities.get(0) : null;
}
return syncRegistrationEntity;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider handling or logging when multiple sync entities are found.

Line 218 silently takes the first entity when findByRegistrationId returns multiple records. If this scenario is possible (e.g., re-processed packets, duplicates), consider logging a warning or adding defensive logic to ensure the most appropriate entity is selected.

🔍 Suggested improvement
 private SyncRegistrationEntity getSyncRegistrationEntity(String registrationId, String workflowInstanceId) {
 	SyncRegistrationEntity syncRegistrationEntity = null;
 	if (workflowInstanceId != null) {
 		syncRegistrationEntity = syncRegistrationService.findByWorkflowInstanceId(workflowInstanceId);
 	}
 	if (syncRegistrationEntity == null) {
 		List<SyncRegistrationEntity> syncRegistrationEntities = syncRegistrationService.findByRegistrationId(registrationId);
-		syncRegistrationEntity = syncRegistrationEntities != null && !syncRegistrationEntities.isEmpty()
-				? syncRegistrationEntities.get(0) : null;
+		if (syncRegistrationEntities != null && !syncRegistrationEntities.isEmpty()) {
+			if (syncRegistrationEntities.size() > 1) {
+				regProcLogger.warn("Multiple sync entities found for registrationId {}, using first", registrationId);
+			}
+			syncRegistrationEntity = syncRegistrationEntities.get(0);
+		}
 	}
 	return syncRegistrationEntity;
 }
🤖 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-processor/pre-processor/registration-processor-supervisor-validator-stage/src/main/java/io/mosip/registration/processor/stages/supervisorvalidator/SupervisorValidationProcessor.java`
around lines 211 - 222, The getSyncRegistrationEntity method silently picks the
first element when syncRegistrationService.findByRegistrationId(...) returns
multiple entries; update getSyncRegistrationEntity to detect when the returned
List has size > 1 and handle it defensively: either log a warning via the
component logger including registrationId and workflowInstanceId, or choose the
most appropriate entity (e.g., the latest by updated/created timestamp) before
returning; reference syncRegistrationService.findByRegistrationId and
syncRegistrationEntities.get(0) to locate the selection point and add the
logging/selection logic there.

Comment on lines +566 to +591
private String resolveAuthenticatedPrincipalName() {
try {
Class<?> holderClass = Class.forName("org.springframework.security.core.context.SecurityContextHolder");
Object context = holderClass.getMethod("getContext").invoke(null);
if (context == null) {
return null;
}
Object authentication = context.getClass().getMethod("getAuthentication").invoke(context);
if (authentication == null) {
return null;
}
Boolean authenticated = (Boolean) authentication.getClass().getMethod("isAuthenticated").invoke(authentication);
if (!Boolean.TRUE.equals(authenticated)) {
return null;
}
String name = (String) authentication.getClass().getMethod("getName").invoke(authentication);
if (name == null || "anonymousUser".equalsIgnoreCase(name)) {
return null;
}
return name;
} catch (ReflectiveOperationException | ClassCastException | LinkageError e) {
regProcLogger.debug(LoggerFileConstant.SESSIONID.toString(), LoggerFileConstant.USERID.toString(), "",
"SyncV3: SecurityContextHolder not available or not authenticated: " + e.getMessage());
return null;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Reflection-based principal resolution is fragile.

The resolveAuthenticatedPrincipalName method uses reflection to access Spring Security's SecurityContextHolder. While defensive (catches all exceptions), this approach is fragile and could break with Spring Security upgrades or ClassLoader changes. Consider injecting an AuthenticationFacade or using Spring Security's @Autowired SecurityContext directly.

💡 Recommended alternative approach

Create an AuthenticationFacade:

`@Component`
public class AuthenticationFacade {
    public String getAuthenticatedUsername() {
        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
        if (authentication == null || !authentication.isAuthenticated()) {
            return null;
        }
        String name = authentication.getName();
        return "anonymousUser".equalsIgnoreCase(name) ? null : name;
    }
}

Then inject and use it in SyncRegistrationServiceImpl:

`@Autowired`(required = false)
private AuthenticationFacade authenticationFacade;

private String resolveAuthenticatedPrincipalName() {
    return authenticationFacade != null ? authenticationFacade.getAuthenticatedUsername() : null;
}
🤖 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-processor/registration-processor-registration-status-service-impl/src/main/java/io/mosip/registration/processor/status/service/impl/SyncRegistrationServiceImpl.java`
around lines 566 - 591, The reflection-based resolveAuthenticatedPrincipalName
in SyncRegistrationServiceImpl is fragile; replace it by creating an
AuthenticationFacade component that exposes getAuthenticatedUsername() which
reads SecurityContextHolder.getContext().getAuthentication(), checks for
null/authenticated and filters "anonymousUser", then inject that facade into
SyncRegistrationServiceImpl (autowire required=false) and rewrite
resolveAuthenticatedPrincipalName to return authenticationFacade != null ?
authenticationFacade.getAuthenticatedUsername() : null; ensure the facade class
name and method (AuthenticationFacade.getAuthenticatedUsername) and the existing
method resolveAuthenticatedPrincipalName are used so callers remain unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant