MOSIP-44098 - Move common JWT decode utilities from EsignetUtil to AdminTestUtil#1908
Merged
mohanachandran-s merged 4 commits intomosip:developfrom Mar 9, 2026
Merged
Conversation
…minTestUtil Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
Contributor
WalkthroughAdded JWT decoding/composition utilities and a PATCH helper that uses role-based bearer tokens in AdminTestUtil. Extended KernelAuthentication to fetch Keycloak tokens via client_credentials and cache tokens per role ("partner" and "mobileauth"). Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase as Test Case
participant AdminUtil as AdminTestUtil
participant KernelAuth as KernelAuthentication
participant ConfigMgr as ConfigManager
participant Keycloak as Keycloak Server
participant TargetAPI as Target API
TestCase->>AdminUtil: patchWithPathParamsBodyHeaderWithBearerToken(...)
AdminUtil->>KernelAuth: getAuthTokenByRole(role)
KernelAuth->>ConfigMgr: getProperty(clientId/clientSecret)
ConfigMgr-->>KernelAuth: credentials
alt Token cached
KernelAuth-->>AdminUtil: cached token
else Token not cached
KernelAuth->>Keycloak: POST TOKEN_URL (client_credentials)
Keycloak-->>KernelAuth: {access_token}
KernelAuth->>KernelAuth: cache token per role
KernelAuth-->>AdminUtil: access_token
end
AdminUtil->>TargetAPI: PATCH with Bearer token + path params + body
TargetAPI-->>AdminUtil: Response
AdminUtil-->>TestCase: Response
sequenceDiagram
participant Caller as Caller
participant AdminUtil as AdminTestUtil
participant JWT as JWT Decoder
Caller->>AdminUtil: decodeAndCombineJwt(jwtString)
AdminUtil->>JWT: split jwtString by '.'
JWT->>AdminUtil: [header, payload, signature]
AdminUtil->>AdminUtil: decodeBase64Url(header)
AdminUtil->>AdminUtil: decodeBase64Url(payload)
AdminUtil->>AdminUtil: combine into {header, payload}
AdminUtil-->>Caller: JSON string with decoded parts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java (1)
287-308: Consider renaming to avoid confusion withgetTokenByRole.This method has a similar name to the existing
getTokenByRole()(line 97) but uses a fundamentally different authentication mechanism (Keycloak client_credentials vs. password/cookie-based). This could lead to developer confusion about which method to use.Additionally, unsupported roles silently return an empty string. Consider adding a log statement:
Proposed improvement
default: + logger.warn("Unsupported role for Keycloak auth: " + role); return ""; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java` around lines 287 - 308, Rename getAuthTokenByRole to a clearer name (e.g., getKeycloakTokenByRole or getClientCredentialsToken) to distinguish it from getTokenByRole, update any callers, and keep the existing logic that uses AdminTestUtil.isValidToken, getAuthTokenFromKeyCloak, partnerCookie and mobileAuthCookie/ConfigManager keys; also replace the silent default return with a logged warning (use your project logger) that includes the unknown role string before returning an empty token so unsupported roles are visible in logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java`:
- Around line 7784-7790: The decodeBase64Url method currently logs the raw
Base64Url segment on exception; update the catch block in
AdminTestUtil.decodeBase64Url to avoid leaking token data by removing the raw
value from the logger call and instead log a redacted or minimal identifier
(e.g., a fixed placeholder like "<redacted-jwt-segment>" or masked string
showing only length or first/last 4 chars) together with the exception; ensure
the logger.error invocation references decodeBase64Url and the redacted
identifier (not the original value) so no raw JWT claims are written to logs.
- Around line 7760-7766: The code calls kernelAuthLib.getAuthTokenByRole(role)
and forwards the returned token into
RestClient.patchWithPathParamsBodyHeaderWithBearerToken even when the token is
empty; update the logic around the token variable to validate the resolved token
(check token is non-null and not empty) after calling getAuthTokenByRole(role)
and before invoking RestClient.patchWithPathParamsBodyHeaderWithBearerToken, and
if validation fails, log a clear error (including role and any kernelAuthLib
error/details) and stop the request path (throw an exception or return an
appropriate error response) so we fail fast instead of sending an Authorization:
Bearer .
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java`:
- Around line 270-285: In getAuthTokenFromKeyCloak, stop logging the full
response body (which can leak ACCESS_TOKEN) and add HTTP status validation and
error handling: check Response.getStatusCode() from sendPostRequest(TOKEN_URL,
params) and if it is not 200 (or expected OK), log a sanitized error containing
the status code and a brief non-sensitive message (no body or tokens) and return
""; otherwise parse the JSON inside a try/catch (catch
JSONException/RuntimeException) and return responseJson.optString(ACCESS_TOKEN,
"") to avoid unhandled parsing exceptions.
- Around line 87-88: The static fields partnerCookie and mobileAuthCookie are
being shared by both password/cookie-based methods (getTokenByRole(),
getAuthForPartner(), getAuthForMobile()) and the Keycloak client_credentials
flow (getAuthTokenByRole()), causing cache collisions; introduce separate static
cache fields for Keycloak tokens (e.g., partnerKeycloakCookie and
mobileKeycloakCookie) and update getAuthTokenByRole() to read/write those new
fields instead of partnerCookie/mobileAuthCookie so the two authentication
mechanisms no longer overwrite each other, ensuring getTokenByRole(),
getAuthForPartner(), and getAuthForMobile() continue to use the original fields
while Keycloak uses the new ones.
- Around line 79-80: The TOKEN_URL construction in KernelAuthentication uses
ConfigManager.getproperty for both "keycloak-external-url" and
"keycloakAuthTokenEndPoint" and may produce a malformed URL when either key is
missing; update KernelAuthentication to validate both values after calling
ConfigManager.getproperty (for "keycloak-external-url" and
"keycloakAuthTokenEndPoint"), and if the endpoint part is empty either supply a
sensible default path (e.g. the expected token endpoint) or throw/configure a
clear initialization error so TOKEN_URL is never set to an incomplete value;
ensure the validation runs at class initialization and surfaces a descriptive
exception or logs an error including the missing key names to aid debugging.
---
Nitpick comments:
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java`:
- Around line 287-308: Rename getAuthTokenByRole to a clearer name (e.g.,
getKeycloakTokenByRole or getClientCredentialsToken) to distinguish it from
getTokenByRole, update any callers, and keep the existing logic that uses
AdminTestUtil.isValidToken, getAuthTokenFromKeyCloak, partnerCookie and
mobileAuthCookie/ConfigManager keys; also replace the silent default return with
a logged warning (use your project logger) that includes the unknown role string
before returning an empty token so unsupported roles are visible in logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 616de046-782e-452a-beab-ea2804a81094
📒 Files selected for processing (2)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.javaapitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java
…minTestUtil Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
jayesh12234
suggested changes
Mar 6, 2026
…minTestUtil Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java`:
- Around line 289-290: Wrap the JSON parsing in KernelAuthentication where you
create new org.json.JSONObject(response.getBody().asString()) in a try-catch
that catches org.json.JSONException (and/or a general Exception) and returns the
empty-string fallback; also pre-check that response.getBody().asString() is
non-null/non-empty before parsing to avoid parsing HTML or empty bodies, and
optionally log the raw response on error for debugging while still returning ""
from the method that currently returns responseJson.optString(ACCESS_TOKEN, "").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b209829-9bec-48e9-9740-33bead39ace2
📒 Files selected for processing (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java (1)
287-288:⚠️ Potential issue | 🟡 MinorGuard JSON parsing before reading
access_token.A successful status code does not guarantee a JSON body.
new org.json.JSONObject(...)can still throw here and bypass the method’s""fallback.🩹 Proposed fix
- org.json.JSONObject responseJson = new org.json.JSONObject(response.getBody().asString()); - return responseJson.optString(ACCESS_TOKEN, ""); + String responseBody = response.getBody().asString(); + if (responseBody == null || responseBody.isBlank()) { + logger.error("Keycloak token response body was empty"); + return ""; + } + try { + org.json.JSONObject responseJson = new org.json.JSONObject(responseBody); + return responseJson.optString(ACCESS_TOKEN, ""); + } catch (org.json.JSONException e) { + logger.error("Failed to parse Keycloak token response", e); + return ""; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java` around lines 287 - 288, The code in KernelAuthentication builds a new org.json.JSONObject from response.getBody().asString() without validating the body, so non-JSON or empty responses can throw and bypass the fallback; update the method to first check response body and/or Content-Type for a JSON payload (e.g., ensure response.getBody() is non-empty and Content-Type contains "application/json"), then parse inside a try/catch that catches org.json.JSONException (and any IO-related exceptions) and returns "" on failure; reference the existing response variable, org.json.JSONObject usage, and the ACCESS_TOKEN constant when extracting the token so the method safely returns the empty-string fallback on malformed or non-JSON responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java`:
- Around line 287-288: The code in KernelAuthentication builds a new
org.json.JSONObject from response.getBody().asString() without validating the
body, so non-JSON or empty responses can throw and bypass the fallback; update
the method to first check response body and/or Content-Type for a JSON payload
(e.g., ensure response.getBody() is non-empty and Content-Type contains
"application/json"), then parse inside a try/catch that catches
org.json.JSONException (and any IO-related exceptions) and returns "" on
failure; reference the existing response variable, org.json.JSONObject usage,
and the ACCESS_TOKEN constant when extracting the token so the method safely
returns the empty-string fallback on malformed or non-JSON responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c092cb0b-185b-4093-a123-a102e4791682
📒 Files selected for processing (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/KernelAuthentication.java
mohanachandran-s
approved these changes
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move common JWT decode utilities from EsignetUtil to AdminTestUtil
Summary by CodeRabbit