-
Notifications
You must be signed in to change notification settings - Fork 119
MOSIP 44129: Created one new Keycloak user 111666. #1873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: SradhaMohanty5899 <mohantysradha10@gmail.com>
Signed-off-by: SradhaMohanty5899 <mohantysradha10@gmail.com>
Signed-off-by: SradhaMohanty5899 <mohantysradha10@gmail.com>
Signed-off-by: SradhaMohanty5899 <mohantysradha10@gmail.com>
WalkthroughAdds external partner authentication support by introducing a new cookie field in the test framework, implementing an external partner authentication method in the kernel utility, and configuring a new IAM user with external credentials in the properties file. Changes
Sequence DiagramsequenceDiagram
actor Test as Test Runner
participant KA as KernelAuthentication
participant EP as Internal Auth Endpoint
Test->>KA: getTokenByRole("partnerauthexternal")
activate KA
KA->>KA: Check cache for token
alt Token not cached
KA->>KA: getAuthForPartnerAuthExternal()
KA->>KA: Build request with APPID, PASSWORD<br/>(external), USER_NAME (external)<br/>+ internal auth payload
KA->>EP: POST auth request
activate EP
EP->>EP: Authenticate external user
EP-->>KA: Return TOKEN
deactivate EP
KA->>KA: Cache TOKEN
end
KA-->>Test: Return cached TOKEN
deactivate KA
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java (1)
355-372: Consider extracting common authentication logic to reduce duplication.The
getAuthForPartnerAuthExternal()method is nearly identical togetAuthForPartnerAuth()(lines 337-353), differing only in the username and password fields. This duplication makes the codebase harder to maintain and increases the risk of inconsistencies.♻️ Refactoring suggestion to eliminate duplication
Consider extracting the common authentication logic into a helper method:
+private String getPartnerAuthInternal(String username, String password) { + JSONObject request = new JSONObject(); + request.put(GlobalConstants.APPID, ConfigManager.getPmsAppId()); + request.put(GlobalConstants.PASSWORD, password); + request.put(GlobalConstants.USER_NAME, username); + JSONObject actualInternalrequest = getRequestJson(authInternalRequest); + request.put(GlobalConstants.CLIENTID, ConfigManager.getPmsClientId()); + request.put(GlobalConstants.CLIENTSECRET, ConfigManager.getPmsClientSecret()); + actualInternalrequest.put(GlobalConstants.REQUEST, request); + Response reponse = AdminTestUtil.postWithJson(authenticationInternalEndpoint, actualInternalrequest); + String responseBody = reponse.getBody().asString(); + return new org.json.JSONObject(responseBody).getJSONObject(dataKey).getString(GlobalConstants.TOKEN); +} + @SuppressWarnings({ "unchecked" }) public String getAuthForPartnerAuth() { - JSONObject request = new JSONObject(); - request.put(GlobalConstants.APPID, ConfigManager.getPmsAppId()); - request.put(GlobalConstants.PASSWORD, partner_password); - request.put(GlobalConstants.USER_NAME, partner_auth_userName); - JSONObject actualInternalrequest = getRequestJson(authInternalRequest); - request.put(GlobalConstants.CLIENTID, ConfigManager.getPmsClientId()); - request.put(GlobalConstants.CLIENTSECRET, ConfigManager.getPmsClientSecret()); - request.put(GlobalConstants.CLIENTID, ConfigManager.getPmsClientId()); - actualInternalrequest.put(GlobalConstants.REQUEST, request); - Response reponse = AdminTestUtil.postWithJson(authenticationInternalEndpoint, actualInternalrequest); - String responseBody = reponse.getBody().asString(); - return new org.json.JSONObject(responseBody).getJSONObject(dataKey).getString(GlobalConstants.TOKEN); + return getPartnerAuthInternal(partner_auth_userName, partner_password); } @SuppressWarnings({ "unchecked" }) public String getAuthForPartnerAuthExternal() { - JSONObject request = new JSONObject(); - request.put(GlobalConstants.APPID, ConfigManager.getPmsAppId()); - request.put(GlobalConstants.PASSWORD, partner_auth_externaluser_password); - request.put(GlobalConstants.USER_NAME, partner_auth_external_userName); - JSONObject actualInternalrequest = getRequestJson(authInternalRequest); - request.put(GlobalConstants.CLIENTID, ConfigManager.getPmsClientId()); - request.put(GlobalConstants.CLIENTSECRET, ConfigManager.getPmsClientSecret()); - request.put(GlobalConstants.CLIENTID, ConfigManager.getPmsClientId()); - actualInternalrequest.put(GlobalConstants.REQUEST, request); - Response reponse = AdminTestUtil.postWithJson(authenticationInternalEndpoint, actualInternalrequest); - String responseBody = reponse.getBody().asString(); - return new org.json.JSONObject(responseBody).getJSONObject(dataKey).getString(GlobalConstants.TOKEN); + return getPartnerAuthInternal(partner_auth_external_userName, partner_auth_externaluser_password); }This refactoring would also:
- Eliminate the duplicate
CLIENTIDassignment present in both methods (lines 347 and 366)- Apply the same pattern to other similar methods like
getAuthForPartner(),getAuthForDeviceProvider(), etc.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.javaapitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.javaapitest-commons/src/main/resources/config/Kernel.properties
🧰 Additional context used
🧬 Code graph analysis (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java (2)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java (1)
AdminTestUtil(133-7447)apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/GlobalConstants.java (1)
GlobalConstants(3-284)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-maven-apitest-commons / maven-build
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (5)
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java (1)
76-76: LGTM! Consistent with existing cookie field pattern.The new
partnerauthexternalCookiefield follows the established pattern for cookie storage in this class and integrates well with the external partner authentication flow added in KernelAuthentication.apitest-commons/src/main/resources/config/Kernel.properties (2)
70-70: LGTM! Role assignment is appropriate.The AUTH_PARTNER role assignment for user 111666 is consistent with other partner authentication users (e.g., 111999 on Line 69).
65-66: List counts are balanced.User count (10) and password count (10) are consistent. Ensure any future modifications maintain this 1:1 correspondence.
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java (2)
41-42: LGTM! Field initialization follows established pattern.The new credential fields are properly initialized from the configuration properties, consistent with other partner credential fields in the class.
129-132: LGTM! Token caching logic is correct.The new "partnerauthexternal" case properly validates token validity before fetching a new token, following the same pattern as other authentication roles.
Created a new Keycloak user with ID 111666 and assigned the AUTH_PARTNER role. However, the user details are not getting stored in the PMS database.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.