1844 - Updated prompt consent handling for Sunbird test cases and enhanced FAPI test cases for MOSIP ID use case#1895
Conversation
…the FAPI test cases for mosipid usecase Signed-off-by: prathmeshj12 <prathmesh.j@cyberpwn.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated test infrastructure and OIDC client configurations to support FAPI/JWE userinfo flows. Added FAPI client patching assets, reorganized test dependency graphs, enhanced test skip logic, refactored OIDC client templates/inputs, and standardized OAuth prompt parameters. ChangesFAPI/JWE User Info Test Infrastructure Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api-test/src/main/resources/esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfo.yml (1)
31-44: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueStatus-code-only validation reduces test coverage.
The new scenario
ESignet_GetOidcUserInfo_MOCK_uin_IdpAccessToken_StatusCode_toUpdatedJWEInfo_Valid_Smoke(and the updated scenario at line 17) now validate only the HTTP status code ("200"), rather than decrypting and validating the JWE userinfo payload. While this simplifies testing for encrypted responses, it does not verify that the returned userinfo contains correct claims, audience, or structure.If JWE decryption/validation is complex, consider at least validating the JWE structure (three dot-separated segments) or adding a separate detailed validation test case.
🤖 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 `@api-test/src/main/resources/esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfo.yml` around lines 31 - 44, The test ESignet_GetOidcUserInfo_MOCK_uin_IdpAccessToken_StatusCode_toUpdatedJWEInfo_Valid_Smoke currently only asserts the HTTP status "200"; update its validation to also verify the JWE userinfo payload from the output template (esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfoResult) by either performing full JWE decryption and claim checks (audience, required claims, structure) using the same key/logic as production or, if decryption is too heavy, at minimum assert the response body is a valid JWE string (three dot-separated segments) and add a follow-up test that decrypts and asserts specific claims (sub, aud, exp) to ensure proper coverage; reference the scenario name and inputTemplate/outputTemplate when making the changes.
🤖 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 `@api-test/src/main/resources/config/testCaseInterDependency_mosip-id.json`:
- Around line 845-847: The dependency list for
TC_ESignet_Oauth_FAPI_forUserInfoJWE_01 includes a redundant direct dependency
on TC_ESignet_CreateOIDCClient_MOSIPID_30 because
TC_ESignet_FAPI_PatchOIDCClientAdditionalConfig_01 already depends on that
creation test; remove TC_ESignet_CreateOIDCClient_MOSIPID_30 from the array for
TC_ESignet_Oauth_FAPI_forUserInfoJWE_01 so it only depends on
TC_ESignet_FAPI_PatchOIDCClientAdditionalConfig_01, leaving the transitive
dependency intact.
In
`@api-test/src/main/resources/esignet/FAPI/FAPIGetUserInfo/FAPIGetUserInfo.yml`:
- Around line 20-36: The current test case
ESignet_GetOidcUserInfo_FAPI_UIN_Dpop_AccessToken_StatusCode_forUserInfoJWE_Valid_Smoke
only asserts HTTP 200; add a new test (or extend this one) that fetches the
response body from endpoint /v1/esignet/oidc/userinfo (inputTemplate
esignet/FAPI/FAPIGetUserInfo/FAPIGetUserInfo), decrypts the JWE using the
server/private key used in your test fixtures, and asserts JWE structure and
expected claims (iss, sub, aud, exp) and audience matching the client_id;
alternatively at minimum validate the response is valid JWE compact/JSON
structure (headers + ciphertext) before keeping the status-only test; reference
uniqueIdentifier TC_ESignet_FAPI_GetUserInfo_02 when adding the new scenario and
reuse dpop_access_token / DPOP_PROOF_WITH_ACCESS_TOKEN from the input to perform
token-bound validation.
In `@api-test/src/main/resources/esignet/OidcClient/OIDCClient.yml`:
- Line 721: The embedded JSON in the OIDC client payload uses unquoted object
keys inside authContextRefs (e.g., {acrValues: "mosip:idp:acr:static-code"}),
which is invalid JSON; update the input payload so each object in
authContextRefs uses quoted keys (e.g., {"acrValues": "..."}), ensuring all
occurrences of authContextRefs/acrValues in the OIDCClient.yml payload strings
are fixed and still properly escaped inside the YAML string.
---
Outside diff comments:
In
`@api-test/src/main/resources/esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfo.yml`:
- Around line 31-44: The test
ESignet_GetOidcUserInfo_MOCK_uin_IdpAccessToken_StatusCode_toUpdatedJWEInfo_Valid_Smoke
currently only asserts the HTTP status "200"; update its validation to also
verify the JWE userinfo payload from the output template
(esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfoResult) by either
performing full JWE decryption and claim checks (audience, required claims,
structure) using the same key/logic as production or, if decryption is too
heavy, at minimum assert the response body is a valid JWE string (three
dot-separated segments) and add a follow-up test that decrypts and asserts
specific claims (sub, aud, exp) to ensure proper coverage; reference the
scenario name and inputTemplate/outputTemplate when making the changes.
🪄 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: 82296bf7-8f79-4afb-a3c7-ad532933da85
📒 Files selected for processing (16)
api-test/pom.xmlapi-test/src/main/java/io/mosip/testrig/apirig/esignet/utils/EsignetUtil.javaapi-test/src/main/resources/config/testCaseInterDependency_mock.jsonapi-test/src/main/resources/config/testCaseInterDependency_mosip-id.jsonapi-test/src/main/resources/config/testCaseInterDependency_sunbirdrc.jsonapi-test/src/main/resources/esignet/EndToEndFlowWithV3MOCK/userinfo/GetOidcUserInfo.ymlapi-test/src/main/resources/esignet/FAPI/FAPIGetUserInfo/FAPIGetUserInfo.ymlapi-test/src/main/resources/esignet/FAPI/FAPIGetUserInfo/FAPIGetUserInfoStatusCodeResult.hbsapi-test/src/main/resources/esignet/FAPI/FAPIPatchClientConfig/FAPIPatchClientConfig.hbsapi-test/src/main/resources/esignet/FAPI/FAPIPatchClientConfig/FAPIPatchClientConfig.ymlapi-test/src/main/resources/esignet/FAPI/OauthFAPI/OauthFAPI.ymlapi-test/src/main/resources/esignet/OidcClient/OIDCClient.ymlapi-test/src/main/resources/esignet/OidcClient/OIDCClientFAPI.hbsapi-test/src/main/resources/esignet/SunBirdC/OAuthDetailsRequestSunBirdC/OAuthDetailsRequestSunBirdC.ymlapi-test/src/main/resources/esignet/SunBirdCNegTC/OAuthDetailsRequestSunBirdCNegTC/OAuthDetailsRequestSunBirdCNegTC.ymlapi-test/testNgXmlFiles/esignetSuite.xml
💤 Files with no reviewable changes (1)
- api-test/src/main/resources/config/testCaseInterDependency_sunbirdrc.json
…the FAPI test cases for mosipid usecase Signed-off-by: prathmeshj12 <prathmesh.j@cyberpwn.com>
…API test cases for MOSIP ID use case Signed-off-by: prathmeshj12 <prathmesh.j@cyberpwn.com>
zesu22
left a comment
There was a problem hiding this comment.
approving because it is already approved by @mohanachandran-s
Updated prompt consent handling for Sunbird test cases and enhanced FAPI test cases for MOSIP ID use case
Summary by CodeRabbit
Tests
Chores