Skip to content

ES-2955 idrepo update identity workaround#193

Merged
ase-101 merged 2 commits intomosip:developfrom
nandhu-kumar:develop-local
Mar 18, 2026
Merged

ES-2955 idrepo update identity workaround#193
ase-101 merged 2 commits intomosip:developfrom
nandhu-kumar:develop-local

Conversation

@nandhu-kumar
Copy link

@nandhu-kumar nandhu-kumar commented Mar 17, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Updated password handling to ensure compatibility with identity-repository version 1.3.0, improving signup/login interoperability and preventing authentication inconsistencies.

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Walkthrough

Added a new value field to the Password DTO and changed the Password constructor parameter order; updated the call site in password generation to match the new constructor ordering for IDRepo 1.3.0 compatibility.

Changes

Cohort / File(s) Summary
Password DTO
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/dto/Password.java
Added private String value; field to the Password class (Lombok-generated accessors/constructors updated).
Password generation / constructor call
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java
Updated Password constructor invocation to new parameter ordering (salt, hashValue) to match modified constructor signature for compatibility with IDRepo 1.3.0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A little field hopped into view,
Value nestled with hash and salt too,
Constructor shuffled its tune,
Call sites updated by noon,
Compatibility bounces anew 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'ES-2955 idrepo update identity workaround' accurately describes the main change: adding a new field and reordering constructor arguments in the Password class for IDRepo 1.3.0 compatibility.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java (1)

386-388: ⚠️ Potential issue | 🔴 Critical

Constructor call needs to be updated to work with the new Password class structure.

Once @NoArgsConstructor is added to Password.java, update this method to use setters, or alternatively pass null as the third argument and set value afterward in buildIdentityRequest.

🐛 Proposed fix: Use setters with `@NoArgsConstructor`
         if (!StringUtils.isEmpty(responseWrapper.getResponse().getHashValue()) &&
                 !StringUtils.isEmpty(responseWrapper.getResponse().getSalt())) {
-            return new Password(responseWrapper.getResponse().getHashValue(),
-                    responseWrapper.getResponse().getSalt());
+            Password password = new Password();
+            password.setHash(responseWrapper.getResponse().getHashValue());
+            password.setSalt(responseWrapper.getResponse().getSalt());
+            return password;
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`
around lines 386 - 388, The Password constructor call in
IdrepoProfileRegistryPluginImpl (the return new
Password(responseWrapper.getResponse().getHashValue(),
responseWrapper.getResponse().getSalt())) no longer matches the updated Password
class; replace it by creating a no-args Password instance and use the setters
(e.g., setHashValue, setSalt, setValue) to populate fields, or if you prefer the
alternative, call the constructor with a null third arg and ensure
buildIdentityRequest sets the value afterward; update the code paths that
consume this Password (including buildIdentityRequest) so they expect the fields
to be set via setters rather than the old two-arg constructor.
mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/dto/Password.java (1)

11-17: ⚠️ Potential issue | 🔴 Critical

Compilation failure: Adding the value field breaks existing 2-argument constructor calls.

The pipeline failure confirms that @AllArgsConstructor now generates only a 3-argument constructor Password(String, String, String), but existing code in generateSaltedHash() (line 387-388) calls the 2-argument constructor.

Add @NoArgsConstructor to enable setter-based construction, or explicitly define a 2-argument constructor for backward compatibility.

🐛 Proposed fix: Add `@NoArgsConstructor` for setter-based usage
 import lombok.AllArgsConstructor;
 import lombok.Data;
+import lombok.NoArgsConstructor;
 
 `@Data`
 `@AllArgsConstructor`
+@NoArgsConstructor
 public class Password {
 
     private String hash;
     private String salt;
     private String value; // Added for compatible with 1.3.0 IDRepo
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/dto/Password.java`
around lines 11 - 17, The new field `value` caused `@AllArgsConstructor` to only
generate a 3-arg ctor which breaks existing 2-arg calls from
generateSaltedHash(); fix by adding Lombok's `@NoArgsConstructor` to the Password
class (above class declaration alongside `@Data` and `@AllArgsConstructor`) so
callers can use setter-based construction, or alternatively add an explicit
two-argument constructor Password(String hash, String salt) to preserve backward
compatibility for generateSaltedHash().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Line 487: The code calls password.getHashValue() which doesn't exist on
Password; instead use the Password.hash accessor and then the inner
PasswordHash.getHashValue(); update the logic in IdrepoProfileRegistryPluginImpl
where password.setValue(...) is called to retrieve the actual hash string via
password.getHash().getHashValue(), adding a null-check for password.getHash()
and handling the absent case appropriately before calling password.setValue;
keep the change localized to the password.setValue call site and ensure any
imports or null-safety follow project conventions.

---

Outside diff comments:
In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/dto/Password.java`:
- Around line 11-17: The new field `value` caused `@AllArgsConstructor` to only
generate a 3-arg ctor which breaks existing 2-arg calls from
generateSaltedHash(); fix by adding Lombok's `@NoArgsConstructor` to the Password
class (above class declaration alongside `@Data` and `@AllArgsConstructor`) so
callers can use setter-based construction, or alternatively add an explicit
two-argument constructor Password(String hash, String salt) to preserve backward
compatibility for generateSaltedHash().

In
`@mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java`:
- Around line 386-388: The Password constructor call in
IdrepoProfileRegistryPluginImpl (the return new
Password(responseWrapper.getResponse().getHashValue(),
responseWrapper.getResponse().getSalt())) no longer matches the updated Password
class; replace it by creating a no-args Password instance and use the setters
(e.g., setHashValue, setSalt, setValue) to populate fields, or if you prefer the
alternative, call the constructor with a null third arg and ensure
buildIdentityRequest sets the value afterward; update the code paths that
consume this Password (including buildIdentityRequest) so they expect the fields
to be set via setters rather than the old two-arg constructor.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d88c47b8-eec4-4d35-a865-645fdc666343

📥 Commits

Reviewing files that changed from the base of the PR and between 4432aee and c5f74f8.

📒 Files selected for processing (2)
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/dto/Password.java
  • mosip-identity-plugin/src/main/java/io/mosip/signup/plugin/mosipid/service/IdrepoProfileRegistryPluginImpl.java

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
@ase-101 ase-101 merged commit 11fea50 into mosip:develop Mar 18, 2026
13 checks passed
KashiwalHarsh pushed a commit to Infosys/esignet-plugins that referenced this pull request Mar 18, 2026
* fix: idrepo update identity workaround

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

* ES-2955 | fix: idrepo update identity workaround

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

---------

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
Prafulrakhade pushed a commit that referenced this pull request Mar 18, 2026
* Update README.md (#187)

Signed-off-by: Rajapandi M <138785181+rajapandi1234@users.noreply.github.com>

* ES-2914 (#190)

* ES-2914

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comments

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comments

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comments

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comments

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

---------

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* ES-2914 (#191)

* Fixed mock identity validation issue removed unnecessary configurations

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Removed unnecessary configurations

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comment

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comment

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* Fixed review comments

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

---------

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

* ES-2955 idrepo update identity workaround (#193)

* fix: idrepo update identity workaround

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

* ES-2955 | fix: idrepo update identity workaround

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

---------

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

* Corrected the property name (#194)

Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>

---------

Signed-off-by: Rajapandi M <138785181+rajapandi1234@users.noreply.github.com>
Signed-off-by: ase-101 <sunkadaeanusha@gmail.com>
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
Co-authored-by: Rajapandi M <138785181+rajapandi1234@users.noreply.github.com>
Co-authored-by: ase-101 <sunkadaeanusha@gmail.com>
Co-authored-by: Nandhukumar <nandhukumare@gmail.com>
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