[ES 2924] Prompt Validation before id token hint to propogate right error code

[KashiwalHarsh]

Summary by CodeRabbit

• Bug Fixes

  • Standardized error responses for ID token hint validation—users will now see consistent "login required" guidance when token validation or audience checks fail.

• Refactor

  • Consolidated ID token hint handling into a single shared flow to simplify and unify authorization behavior across endpoints.

• Tests

  • Updated authorization tests to reflect the new error behavior and centralized handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1655ef73-c375-4c0e-9e8a-31b2b542ae7d

📥 Commits

Reviewing files that changed from the base of the PR and between 0f62647 and 36e3c0f.

📒 Files selected for processing (1)

• oidc-service-impl/src/test/java/io/mosip/esignet/services/AuthorizationServiceTest.java

---

Walkthrough

All OAuth detail endpoints (V2, V3, PAR) now route through buildTransactionAndOAuthDetailResponse which conditionally processes id_token_hint; id_token_hint validation failures now throw LOGIN_REQUIRED. Tests updated to reflect the new error semantics and required mocks.

Changes

Centralize id_token_hint processing with error code alignment

Layer / File(s)	Summary
Error semantics alignment in id_token_hint validation oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationHelperService.java	validateAndGetSubjectAndNonce throws LOGIN_REQUIRED when JWT parsing or audience/clientId validation fails, replacing the previous INVALID_ID_TOKEN_HINT exception.
Centralized id_token_hint routing through shared builder oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java	getOauthDetailsV2, getOauthDetailsV3, and getPAROAuthDetails now delegate to buildTransactionAndOAuthDetailResponse; when idTokenHint is provided the builder invokes handleIdTokenHint (casting to OAuthDetailRequestV3) and overrides the transaction nonce and state.
Test expectations for id_token_hint and error semantics oidc-service-impl/src/test/java/io/mosip/esignet/services/AuthorizationServiceTest.java	Multiple getOauthDetailsV3 tests now mock ClientDetail and nonce/auth-factor resolution and assert LOGIN_REQUIRED instead of INVALID_ID_TOKEN_HINT for invalid/missing hints and audience mismatches.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hint once scattered, now finds the gate,
One builder listens, decides a fate.
When tokens falter, the login bell peals,
Nonce and state set — shared pathways heal.

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: modifying error-handling behavior in id_token_hint validation to propagate the correct error code (LOGIN_REQUIRED instead of INVALID_ID_TOKEN_HINT).
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.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 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
`@oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java`:
- Around line 518-520: The cast to OAuthDetailRequestV3 in
AuthorizationServiceImpl when idTokenHint != null is unsafe; before casting
oauthDetailReqDto, verify its type (e.g., oauthDetailReqDto instanceof
OAuthDetailRequestV3) and handle the mismatch by either throwing a clear
IllegalArgumentException/BadRequest or falling back to a safe path, then call
handleIdTokenHint((OAuthDetailRequestV3) oauthDetailReqDto, httpServletRequest)
only after the check; reference oauthDetailReqDto, OAuthDetailRequestV3,
handleIdTokenHint and the surrounding conditional to locate where to add the
defensive check and error handling.

In
`@oidc-service-impl/src/test/java/io/mosip/esignet/services/AuthorizationServiceTest.java`:
- Around line 1166-1171: The test is ambiguous because it both asserts a
non-null OAuthDetailResponseV2 and catches EsignetException expecting
ErrorConstants.LOGIN_REQUIRED; since the JWT audience
("mosip-signup-oauth-client") does not match oauthDetailRequest.clientId
("34567") the test should explicitly expect failure: replace the try/catch block
around AuthorizationServiceTest.getOauthDetailsV3(oauthDetailRequest, request)
with an assertion that the call throws EsignetException (e.g., assertThrows) and
then assert the thrown exception's errorCode equals
ErrorConstants.LOGIN_REQUIRED; alternatively, if you intend success, change the
test fixture so oauthDetailRequest.clientId matches the JWT audience before
calling getOauthDetailsV3.
- Around line 728-755: The test currently fails to reach the cookie-null branch
in AuthorizationServiceImpl#handleIdTokenHint because
authorizationHelperService.validateAndGetSubjectAndNonce(...) is causing an
earlier failure; update the test to mock
authorizationHelperService.validateAndGetSubjectAndNonce(...) to return a valid
subject/nonce (or otherwise not throw) so getOauthDetailsV3(...) proceeds to the
cookie check, then change the asserted error to
ErrorConstants.INVALID_ID_TOKEN_HINT (or if you meant to test the earlier
validation, instead keep the existing mocks and assert
ErrorConstants.LOGIN_REQUIRED). Ensure you reference
authorizationHelperService.validateAndGetSubjectAndNonce,
AuthorizationServiceImpl.handleIdTokenHint, getOauthDetailsV3, and
ErrorConstants.INVALID_ID_TOKEN_HINT when making the change.

🪄 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: e128ac83-a8f8-4a33-9216-4daf10de0282

📥 Commits

Reviewing files that changed from the base of the PR and between 4418395 and 0f62647.

📒 Files selected for processing (3)

• oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationHelperService.java
• oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java
• oidc-service-impl/src/test/java/io/mosip/esignet/services/AuthorizationServiceTest.java

[coderabbitai]

Actionable comments posted: 0