Skip to content

DSD-10748: fix internal OTP partner identity after service merge - #1786

Open
nagendra0721 wants to merge 1 commit into
mosip:develop-1764from
nagendra0721:fix/1764-internal-otp-partner-identity
Open

nagendra0721 wants to merge 1 commit into
mosip:develop-1764from
nagendra0721:fix/1764-internal-otp-partner-identity

Conversation

@nagendra0721

@nagendra0721 nagendra0721 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Internal OTP requests can fail with IDA-OTA-011 after the service merge: OTP transaction creation uses the primary security manager and stores the configured IDA client ID, while internal authentication validates against the authenticated caller identity. This blocks Resident VID creation and auth lock/unlock requests.

Inject internalAuthSecurityManager separately and use it only when building internal OTP transactions. External OTP transactions retain their existing security manager, and partner-ID validation is unchanged.

Validation:

  • A temporary regression test reproduced IDA-OTA-011 before the fix and passed unchanged after it. Additional temporary tests covered a different caller being rejected, usernames without the service-account prefix, and external OTP partner identity; these tests were removed after verification.
  • Clean package build on this PR branch passed: 58 existing tests passed, 3 skipped, zero failures or errors.
  • Environment API suite validation is pending deployment.
mvn -o -pl authentication-service -am clean package -Dtest=OTPServiceImplTest,OTPAuthServiceTest,AuthTransactionBuilderTest,InternalOTPControllerTest,InternalAuthControllerTest,OTPControllerTest -Dsurefire.failIfNoSpecifiedTests=false -DskipTests=false -Dmaven.javadoc.skip=true -Dgpg.skip=true -Dmaven.gitcommitid.skip=true

Summary by CodeRabbit

  • Bug Fixes
    • Improved OTP transaction processing for internal authentication requests.
    • Internal and external authentication flows now use the appropriate security handling, improving transaction reliability and consistency.

Signed-off-by: nagendra0721 <nagendra0718@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: ee34ec85-1759-48b9-997d-efe6fa0d46a8

📥 Commits

Reviewing files that changed from the base of the PR and between 6d14fd7 and eb09c04.

📒 Files selected for processing (1)
  • authentication/authentication-common/src/main/java/io/mosip/authentication/common/service/impl/OTPServiceImpl.java

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

OTPServiceImpl now injects an internal security manager. The saveTxn method selects it for internal requests and retains the existing security manager for other requests.

Changes

OTP security selection

Layer / File(s) Summary
Security manager wiring and transaction creation
authentication/authentication-common/src/main/java/io/mosip/authentication/common/service/impl/OTPServiceImpl.java
OTPServiceImpl injects internalAuthSecurityManager. saveTxn passes the internal or external security manager to AuthTransactionBuilder.build(...) based on isInternal.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to eb09c

Internal OTP transaction creation now records identity through the internal security manager, while external OTP behavior remains unchanged. The supplied build and regression results show no current merge-blocking risk.

Poem

Internal paths choose their key
External flows keep consistency
The builder receives the right guard
OTP transactions stay on their shard
Two managers, one clear decree

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 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 internal OTP partner identity fix and the service merge context. It matches the main change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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