MOSIP-43845 : Added API Integration to Esignet UI Automation for OIDC Client Setup#1584
MOSIP-43845 : Added API Integration to Esignet UI Automation for OIDC Client Setup#1584BhuvanaShreeBS wants to merge 11 commits intomosip:developfrom
Conversation
WalkthroughAdds a large ui-test suite: biometric fixtures and certs, extensive configuration/properties, new annotations/models and UIN/VID pool managers, WebDriver capability updates, Esignet utilities (PAR/JWK/OAuth, UI‑spec parsing), many TestNG test scripts/templates, a resource loader, and runner/BaseTest lifecycle changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(220,240,255,0.5)
participant Test as TestScript
participant Runner as Runner
participant Util as EsignetUtil
participant KC as Keycloak/Auth
participant Esignet as Esignet API
participant IDR as ID Repository
end
Note over Runner,Test: Runner starts data-driven test execution
Test->>Util: prepare input (templates + inputstringKeyWordHandler)
Util->>KC: sign client assertion / request token (JWK/DPoP)
KC-->>Util: access token / PAR response
Util->>Esignet: POST PAR or API request with token/cookie
Esignet-->>Util: API response
alt real-id flow
Util->>IDR: request UIN/RID
IDR-->>Util: returns UIN/RID
Util-->>Test: response + generated ids
else mock flow
Util-->>Test: mock response
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 |
BhuvanaShreeBS
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ui-test/src/main/java/base/BaseTest.java (3)
70-70: Same method naming issue on line 70.
250-250: Another instance of the same issue.Line 250 also uses
getproperty.
69-69: Correct method name: UsegetPropertywith uppercase 'P'.Lines 69-70 call
EsignetConfigManager.getproperty()with lowercase 'p', but the method is defined asgetProperty()with uppercase 'P'. This causes a compilation error and must be corrected in both calls:getproperty("runOnBrowserStack")andgetproperty("runMultipleBrowsers").
🟠 Major comments (15)
ui-test/src/main/resources/krb5.conf-18-18 (1)
18-18: Major: Hardcoded Azure hostname breaks portability.The Kerberos realm configuration is hardcoded to a specific Azure environment (
NODE-MASTER.SOUTHINDIA.CLOUDAPP.AZURE.COM). This will fail when tests run in different environments (CI/CD, local development, other cloud regions).Consider externalizing this configuration through:
- Environment variables or system properties
- A separate environment-specific configuration file
- Template substitution during build/deployment
Also applies to: 22-29
ui-test/pom.xml-49-53 (1)
49-53: Update SnakeYAML from version 1.23 to 2.0 or later.SnakeYAML 1.23 is vulnerable to CVE-2017-18640, and using SnakeYaml 1.x versions can lead to unnecessary security issues if you directly or indirectly accept yaml files from outside sources. More critically, SnakeYaml versions before 2.0 are vulnerable to CVE-2022-1471, a deserialization vulnerability that can lead to arbitrary code execution. SnakeYaml 2.0 was released in early 2023 to mitigate the default behavior that can lead to possible arbitrary code execution, and in this version, the constructor that every new yaml() uses now extends SafeConstructor. The latest version is 2.5.
ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/requestApiKey.hbs-2-2 (1)
2-2: Replace hardcoded string literals with placeholders.The
idandversionfields contain the literal string"string", which will be sent as-is in the request payload. These should likely be placeholders like{{id}}and{{version}}to allow dynamic values during test execution.🔎 Apply this diff if id and version should be dynamic:
{ - "id": "string", + "id": "{{id}}", "metadata": {}, "request": { "policyName":"{{policyName}}", "useCaseDescription":"{{useCaseDescription}}" }, "requesttime": "{{requesttime}}", - "version": "string" + "version": "{{version}}" }Also applies to: 9-9
ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/RequestAPIKey.yml-2-2 (1)
2-2: Resolve inconsistency between test name and expected outcome.The test is named
EsignetUI_SubmitPartnerApiKeyRequest_Rejection_All_Valid_Smoke_sidand the description states "Submit partner Api Key request rejection all valid details", but the expected output is a success message:"Policy mapping request submitted successfully.".This creates confusion about the test's intent:
- If testing successful submission, rename to remove "Rejection" (e.g.,
EsignetUI_SubmitPartnerApiKeyRequest_All_Valid_Smoke_sid)- If testing rejection, update the expected output to an appropriate error message
Also applies to: 5-5, 16-16
ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/requestAPIKeyResult.hbs-1-5 (1)
1-5: Fix structural mismatch between template and test output definition.The template generates
{"response": {"message": "..."}}, but the YAML output atRequestAPIKey.yml(lines 15-17) expects{"message": "..."}without theresponsewrapper. This inconsistency will cause test failures. Either remove theresponsewrapper from the template or update the YAML output to include it. Other similar templates in the repository (e.g.,uploadCertificateResult.hbs) exhibit the same pattern mismatch, suggesting a systematic issue requiring alignment across all PmsIntegration test definitions.ui-test/src/main/java/runners/Runner.java-56-56 (1)
56-56: Consider thread-safety for the skipAll flag.The
skipAllfield is a public static mutable boolean that can be modified by any thread. In a parallel test execution environment (as configured in the data provider), this could lead to race conditions.Consider using
AtomicBooleanor a thread-safe alternative if this flag needs to be shared across threads, or make it thread-local if it should be per-thread.ui-test/src/main/java/utils/LanguageUtil.java-19-19 (1)
19-19: Avoid exposing mutable internal state directly.Making
languagesMappublic exposes a mutableHashMapthat can be modified by external code, potentially causing unexpected behavior across tests.🔎 Apply this diff to expose an unmodifiable view instead:
- public static final Map<String, String> languagesMap = new HashMap<>(); + private static final Map<String, String> languagesMap = new HashMap<>(); private static final Map<String, String> langCodeMappingMap = new HashMap<>(); public static List<String> supportedLanguages = new ArrayList<>();Then add a getter method after the static block:
/** * Returns an unmodifiable view of the languages map. */ public static Map<String, String> getLanguagesMap() { return Collections.unmodifiableMap(languagesMap); }Committable suggestion skipped: line range outside the PR's diff.
ui-test/Biometric Devices/Iris/Single/DigitalId.json-9-9 (1)
9-9: Populate emptydateTimefield with valid ISO 8601 timestamp.The
dateTimefield is empty across all DigitalId.json test fixtures (Iris, Finger, Face variants). This is inconsistent with the expected timestamp format shown in adminQueries.properties:"2025-01-01T00:00:00.000+05:30". Populate each file with a valid ISO 8601 datetime with timezone offset to match real SBI device data patterns.ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicyGroup/definePolicyGroupResult.hbs-3-3 (1)
3-3: Add quotes around the template variable to ensure valid JSON.The
{{is_Active}}placeholder lacks quotes, which will produce invalid JSON unless it renders as a JSON boolean literal (true/false). If the variable is undefined, null, or a string, the output will be malformed.🔎 Apply this diff to ensure valid JSON output:
- "is_Active": {{is_Active}} + "is_Active": "{{is_Active}}"Alternatively, if
is_Activeis guaranteed to be a boolean, add a comment documenting this requirement.ui-test/src/main/java/models/Uin.java-4-10 (1)
4-10: Add initialization for phone and email fields.The constructor only initializes the
uinfield, leavingphoneand🔎 Consider this fix to add setters:
public String getEmail() { return email; } + + public void setPhone(String phone) { + this.phone = phone; + } + + public void setEmail(String email) { + this.email = email; + }Alternatively, expand the constructor:
- public Uin(String uin) { + public Uin(String uin, String phone, String email) { this.uin = uin; + this.phone = phone; + this.email = email; }ui-test/src/main/resources/config.properties-53-68 (1)
53-68: Hardcoded secrets committed to version control.Static analysis has flagged multiple hardcoded credentials (lines 57, 61-67). Even for test environments, committing secrets poses security and compliance risks:
- Secrets may be reused across environments
- Git history retains secrets even after removal
- Violates least-privilege and secret management best practices
Consider:
- Using environment variables or a secrets manager
- Keeping the values empty with instructions to populate locally
- Using
.properties.examplepattern with placeholdersui-test/src/main/java/utils/EsignetUtil.java-53-54 (1)
53-54: Consider encapsulation and thread safety for shared static fields.The public static mutable fields
pluginNameandsignupActiveProfilesare accessible and modifiable from anywhere, which can lead to race conditions if tests run in parallel. Consider making these private with synchronized access methods or using thread-safe alternatives.ui-test/src/main/java/utils/EsignetUtil.java-335-348 (1)
335-348: Lazy initialization race condition on shared static field.The
signupUISpecResponseis lazily initialized without synchronization. Multiple threads could trigger duplicate HTTP requests. Consider using double-checked locking withvolatileor eager initialization.🔎 Apply this diff to add synchronization:
- private static JSONObject signupUISpecResponse; + private static volatile JSONObject signupUISpecResponse; private static JSONObject getSignupUISpecResponse() { if (signupUISpecResponse == null) { - try { - logger.info("Loading Signup UI Spec from " + UiConstants.SIGNUP_UI_SPEC_URL); - Response response = RestClient.getRequest(UiConstants.SIGNUP_UI_SPEC_URL, MediaType.APPLICATION_JSON, - MediaType.APPLICATION_JSON); - signupUISpecResponse = new JSONObject(response.getBody().asString()); - } catch (Exception e) { - logger.error("Failed to load Signup UI Spec from URL.", e); - signupUISpecResponse = new JSONObject(); - } + synchronized (EsignetUtil.class) { + if (signupUISpecResponse == null) { + try { + logger.info("Loading Signup UI Spec from " + UiConstants.SIGNUP_UI_SPEC_URL); + Response response = RestClient.getRequest(UiConstants.SIGNUP_UI_SPEC_URL, MediaType.APPLICATION_JSON, + MediaType.APPLICATION_JSON); + signupUISpecResponse = new JSONObject(response.getBody().asString()); + } catch (Exception e) { + logger.error("Failed to load Signup UI Spec from URL.", e); + signupUISpecResponse = new JSONObject(); + } + } + } } return signupUISpecResponse; }Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/utils/EsignetUtil.java-212-217 (1)
212-217: Lazy initialization of shared static field without synchronization.The
signupActuatorResponseArrayis lazily initialized in a non-synchronized context. If multiple threads call this method simultaneously, it could result in duplicate HTTP requests or race conditions. Consider using synchronization or eager initialization.🔎 Apply this diff to add synchronization:
- try { - // Fetch the actuator response array if not already populated - if (signupActuatorResponseArray == null) { - Response response = RestClient.getRequest(url, MediaType.APPLICATION_JSON, MediaType.APPLICATION_JSON); - JSONObject responseJson = new JSONObject(response.getBody().asString()); - signupActuatorResponseArray = responseJson.getJSONArray("propertySources"); - } + try { + // Fetch the actuator response array if not already populated + synchronized (EsignetUtil.class) { + if (signupActuatorResponseArray == null) { + Response response = RestClient.getRequest(url, MediaType.APPLICATION_JSON, MediaType.APPLICATION_JSON); + JSONObject responseJson = new JSONObject(response.getBody().asString()); + signupActuatorResponseArray = responseJson.getJSONArray("propertySources"); + } + }Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/utils/EsignetUtil.java-707-728 (1)
707-728: Static token cache fields lack synchronization for parallel test execution.The static fields
partnerCookieandmobileAuthCookie(lines 64-65) are accessed and modified ingetAuthTokenByRolewithout synchronization. When tests run in parallel within the same JVM process using threads sharing the same memory space, a static variable can be changed by one thread while another is midway through checking it, creating a race condition. The method has a check-then-act vulnerability:if (!AdminTestUtil.isValidToken(partnerCookie))followed by assignment creates a window where concurrent threads can both fetch tokens. Synchronize the method with thesynchronizedkeyword or useAtomicReferencefor thread-safe token caching.
🟡 Minor comments (8)
ui-test/Biometric Devices/Finger/Single/DeviceInfo.json-12-12 (1)
12-12: Fix deviceCode inconsistency.The
deviceCodecontains "slap" but this fixture is for a Single finger device. The firmware field correctly states "MOSIP.SINGLE.1.0.0.0", so the deviceCode should also contain "single" for consistency.🔎 Apply this suggestion to fix the inconsistency:
- "deviceCode": "b692b595-3523-slap-99fc-bd76e35f290f", + "deviceCode": "b692b595-3523-single-99fc-bd76e35f290f",ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/PublishPolicy.yml-4-4 (1)
4-4: Correct the mismatched uniqueIdentifier.The
uniqueIdentifieris set toDependent_Idrepo_createPublishPolicy_01, which doesn't align with the test case nameEsignetUI_PublishPolicy_All_Valid_Smoke. This appears to be a copy-paste oversight and may cause confusion in test tracking or dependency resolution.🔎 Apply this diff to align the identifier:
- uniqueIdentifier: Dependent_Idrepo_createPublishPolicy_01 + uniqueIdentifier: Dependent_Idrepo_EsignetUI_PublishPolicy_All_Valid_Smokeui-test/src/main/resources/config/application.properties-101-107 (1)
101-107: These hardcoded test tokens should be generated dynamically or clearly marked as test fixtures.The values at lines 101-107 are test-only (confirmed by file location and non-usage in production code), but the file already contains a developer note ("## Need to revisit these propeties") indicating they need attention. The codeChallenge and codeVerifier values are generated dynamically in other test contexts (Postman collections, performance-test scripts), so they should either be:
- Generated dynamically during test execution (matching the pattern used elsewhere)
- Replaced with a clear comment explicitly marking them as static test fixtures for configuration initialization
The XSRFTOKEN value appears to be unused and could be removed.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java-113-113 (1)
113-113: Variable naming typo.Same typo as in SimplePost.java:
ouputValidshould beoutputValid.🔎 Apply this diff to fix the typo:
- Map<String, List<OutputValidationDto>> ouputValid = null; + Map<String, List<OutputValidationDto>> outputValid = null;Note: Update references on lines 119, 120, 122, 126, and 127.
Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.java-101-101 (1)
101-101: Variable naming typo.The variable name
ouputValidshould beoutputValid(missing 't').🔎 Apply this diff to fix the typo:
- Map<String, List<OutputValidationDto>> ouputValid = null; + Map<String, List<OutputValidationDto>> outputValid = null;Note: You'll also need to update references to this variable on lines 107, 108, 115, and 117.
Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenIdForUrlEncoded.java-101-101 (1)
101-101: Variable naming typo.Same typo:
ouputValidshould beoutputValid.🔎 Apply this diff:
- Map<String, List<OutputValidationDto>> ouputValid = OutputValidationUtil + Map<String, List<OutputValidationDto>> outputValid = OutputValidationUtilNote: Update references on lines 103 and 104.
Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java-89-89 (1)
89-89: Variable naming typo.
ouputValidshould beoutputValid.🔎 Apply this diff:
- Map<String, List<OutputValidationDto>> ouputValid = null; + Map<String, List<OutputValidationDto>> outputValid = null;Note: Update references on lines 95, 96, 98, 103, and 105.
Committable suggestion skipped: line range outside the PR's diff.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java-89-89 (1)
89-89: Variable naming typo.
ouputValidshould beoutputValid.🔎 Apply this diff:
- Map<String, List<OutputValidationDto>> ouputValid = null; + Map<String, List<OutputValidationDto>> outputValid = null;Note: Update references on lines 95, 96, 98, 102, and 104.
Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (24)
ui-test/src/main/resources/krb5.conf (1)
10-10: Consider reducing KDC timeout for faster test feedback.The 30-second KDC timeout may cause slow test execution when authentication fails. For UI automation tests, a shorter timeout (e.g., 5-10 seconds) would provide faster feedback on failures.
ui-test/src/main/resources/customize-emailable-report-template.html (2)
4-5: Consider adding Subresource Integrity (SRI) for CDN resources.Loading external scripts without SRI hashes makes the application vulnerable to CDN compromises or MITM attacks. While this is a test report template, adding SRI improves security posture.
You can generate SRI hashes at https://www.srihash.org/ or use:
#!/bin/bash curl -s https://www.gstatic.com/charts/loader.js | openssl dgst -sha384 -binary | openssl base64 -A
2-3: Add HTML5 DOCTYPE and metadata for better compatibility.The template is missing standard HTML5 declarations that ensure proper rendering and character encoding.
🔎 Consider adding these declarations:
- +<!DOCTYPE html> <html> <head> +<meta charset="UTF-8"> +<meta name="viewport" content="width=device-width, initial-scale=1.0"> +<title>Test Report</title> <script type="text/javascript"ui-test/pom.xml (1)
89-94: Exclusion enforces the outdated SnakeYAML version.This exclusion ensures that the directly declared SnakeYAML 1.23 (lines 49-53) is used instead of the transitive version from extentreports-cucumber7-adapter. While this pattern is valid for dependency version management, it reinforces the security concern raised about using version 1.23.
ui-test/src/main/java/runners/Runner.java (1)
148-162: Simplify language configuration parsing.The manual string splitting logic for
runLanguagecould be simplified.🔎 Consider using a more concise approach:
- if (runLang != null && !runLang.trim().isEmpty()) { - LOGGER.info("Using runLanguage from config: " + runLang); - // split by comma and trim spaces - String[] langs = runLang.split(","); - for (String lang : langs) { - if (!lang.trim().isEmpty()) { - languages.add(lang.trim()); - } - } - } else { + if (runLang != null && !runLang.trim().isEmpty()) { + LOGGER.info("Using runLanguage from config: " + runLang); + languages = Arrays.stream(runLang.split(",")) + .map(String::trim) + .filter(lang -> !lang.isEmpty()) + .collect(Collectors.toList()); + } else { LOGGER.info("No runLanguage in config, loading from LanguageUtil"); languages = LanguageUtil.supportedLanguages; }ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/publishPolicyMispResult.hbs (1)
1-7: Consider consistent naming convention.The template uses mixed naming conventions:
policyGroupStatusandpolicyTypeuse camelCase, whileis_Activeuses snake_case. Unless this matches the backend API response format, consider standardizing to camelCase (e.g.,isActive) for consistency.🔎 Apply this diff if the naming should be consistent:
{ "response": { "policyGroupStatus": "{{policyGroupStatus}}", "policyType": "{{policyType}}", - "is_Active": "{{is_Active}}" + "isActive": "{{isActive}}" } }ui-test/src/main/resources/config/AuthPolicy6.json (1)
36-36: Fix inconsistent indentation.Line 36 uses tabs while surrounding lines use spaces, creating mixed indentation.
🔎 Apply this diff to use consistent spacing:
- "eng" + "eng"ui-test/src/main/java/models/Vid.java (1)
3-10: Consider adding setters for phone and email fields.The constructor only initializes the
vidfield, leavingphoneandnull. Since getters are provided for these fields, consider adding corresponding setters to allow these values to be populated after construction.🔎 Add setter methods:
public Vid(String vid) { this.vid = vid; } public String getVid() { return vid; } + public void setPhone(String phone) { + this.phone = phone; + } + + public void setEmail(String email) { + this.email = email; + } + public String getPhone() { return phone; }ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/UploadCertificate.yml (1)
21-21: Update test description to be more specific.The description "upload CACertificate with all valid details" is identical to the CA certificate test above. Consider updating it to clarify that this test uploads an intermediate certificate.
🔎 Suggested fix:
- description: upload CACertificate with all valid details + description: upload intermediate certificate with all valid detailsui-test/src/main/resources/config/AuthPolicy2.json (1)
39-42: Fix inconsistent indentation.Line 41 uses tabs while surrounding lines use spaces, creating inconsistent formatting. Standardize to spaces for consistency with the rest of the file.
🔎 Apply this diff to fix the indentation:
"kycLanguages": [ "ara", - "eng" - ], + "eng" + ],ui-test/src/main/java/utils/ResourceBundleLoader.java (2)
17-34: Thread-safety issue with shared map access.The
resourceBundleMap(a non-thread-safeHashMap) is read viagetOrDefault()on line 33 outside the synchronized block, whileloadResourceBundleJson()may be concurrently modifying it (clear + put operations). This can causeConcurrentModificationExceptionor inconsistent reads.Additionally,
loadedLanguage(line 20) should bevolatilesince it's read outside the synchronized block.🔎 Suggested fix using ConcurrentHashMap:
-private static final Map<String, String> resourceBundleMap = new HashMap<>(); +private static final Map<String, String> resourceBundleMap = new java.util.concurrent.ConcurrentHashMap<>(); private static final Logger logger = Logger.getLogger(ResourceBundleLoader.class); private static volatile boolean loaded = false; -private static String loadedLanguage = ""; +private static volatile String loadedLanguage = "";Alternatively, move the
getOrDefault()call inside the synchronized block after the load check.
54-59: Consider adding timeout for HTTP requests.The
downloadJsonmethod opens a URL stream without a timeout. If the locale server is slow or unresponsive, this can block test execution indefinitely.🔎 Example with timeout:
private static String downloadJson(String url) throws IOException { URI uri = URI.create(url); - try (InputStream in = uri.toURL().openStream()) { + java.net.URLConnection conn = uri.toURL().openConnection(); + conn.setConnectTimeout(10_000); // 10 seconds + conn.setReadTimeout(30_000); // 30 seconds + try (InputStream in = conn.getInputStream()) { return new String(in.readAllBytes(), StandardCharsets.UTF_8); } }ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java (1)
1-1: Consider extracting common test scaffolding to reduce duplication.All six test script classes (SimplePost, SimplePostForAutoGenId, SimplePostForAutoGenIdForUrlEncoded, PutWithPathParamsAndBody, AddIdentity, PostWithOnlyPathParam) share nearly identical structure:
setLogLevel()implementationgetTestName()implementationgetTestCaseList()data provider patternsetResultTestName()reflection-based namingConsider creating an abstract base test class that provides this common scaffolding, allowing each test class to focus only on its unique test execution logic. This would improve maintainability and reduce the risk of inconsistent changes across test files.
ui-test/src/main/java/utils/EsignetUtil.java (6)
70-77: Declare configuration constants asfinal.The static fields
display,responseType,client_assertion_type,claim_locales,scope,state,prompt, andaud_keyappear to be constants. Declare them asfinalto prevent accidental modification and clarify intent.🔎 Apply this diff to make fields final:
- static String display = "popup"; - static String responseType = "code"; - static String client_assertion_type = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; - static String claim_locales = "en"; - static String scope = "openid profile"; - static String state = "eree2311"; - static String prompt = "login"; - static String aud_key = "pushed_authorization_request_endpoint"; + static final String display = "popup"; + static final String responseType = "code"; + static final String client_assertion_type = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; + static final String claim_locales = "en"; + static final String scope = "openid profile"; + static final String state = "eree2311"; + static final String prompt = "login"; + static final String aud_key = "pushed_authorization_request_endpoint";
270-289: Add error handling for unexpected regex format.The method assumes the regex contains
{min,max}format. If the regex from the actuator has an unexpected format, lines 274-277 could throwStringIndexOutOfBoundsExceptionorNumberFormatException. Consider adding validation or try-catch to handle malformed regex gracefully.🔎 Example defensive handling:
public static String generateMobileNumberFromRegex() { String regex = getValueFromSignupActuator("applicationConfig: [classpath:/application-default.properties]", "mosip.signup.identifier.regex"); - - String digitRange = regex.substring(regex.indexOf('{') + 1, regex.indexOf('}')); - String[] parts = digitRange.split(","); - int min = Integer.parseInt(parts[0].trim()); - int max = Integer.parseInt(parts[1].trim()); + + try { + int start = regex.indexOf('{'); + int end = regex.indexOf('}'); + if (start == -1 || end == -1 || start >= end) { + logger.error("Invalid regex format: " + regex); + return "1234567890"; // fallback + } + String digitRange = regex.substring(start + 1, end); + String[] parts = digitRange.split(","); + if (parts.length != 2) { + logger.error("Invalid digit range in regex: " + digitRange); + return "1234567890"; // fallback + } + int min = Integer.parseInt(parts[0].trim()); + int max = Integer.parseInt(parts[1].trim()); + } catch (Exception e) { + logger.error("Error parsing regex: " + regex, e); + return "1234567890"; // fallback + }
423-447: Reduce code duplication betweenextractMaxLengthandextractMinLength.These methods share nearly identical logic. Consider extracting common parsing logic into a shared helper method to improve maintainability.
🔎 Example refactoring:
private static int extractLength(String regex, boolean isMax) { if (regex == null) return isMax ? 10 : 2; int start = regex.indexOf('{'); int end = regex.indexOf('}'); if (start != -1 && end != -1) { String[] parts = regex.substring(start + 1, end).split(","); if (parts.length == 2) return Integer.parseInt(parts[isMax ? 1 : 0].trim()); return Integer.parseInt(parts[0].trim()); } return isMax ? 10 : 2; } public static int extractMaxLength(String regex) { return extractLength(regex, true); } public static int extractMinLength(String regex) { return extractLength(regex, false); }
477-488: MakeRegisteredDetailsa static inner class.The non-static inner class
RegisteredDetailscontains only static members, which is an unusual pattern. Declare it asstatic class RegisteredDetailsto clarify that it doesn't need an instance of the outer class and improve code clarity.🔎 Apply this diff:
- public class RegisteredDetails { + public static class RegisteredDetails { private static String registeredMobileNumber;
510-510: Typo in method name:inputstringKeyWordHandeler.The method name contains a typo: "Handeler" should be "Handler". Consider renaming to
inputstringKeyWordHandlerfor consistency.
783-811: Consider extracting PAR request body construction to improve readability.The
generateParRequestUrimethod builds a complex JSON request body inline. Consider extracting this to a separate method (e.g.,buildParRequestBody) to improve readability and make it easier to test or reuse the request body construction logic.ui-test/src/main/resources/esignetUI/OidcClient/OIDCClient.hbs (1)
8-16: Arrays with single placeholders cannot handle multiple values.The
redirectUris,grantTypes, andclientAuthMethodsarrays each contain a single string placeholder. This design prevents passing multiple values, which are commonly needed for OIDC clients (e.g., multiple redirect URIs or grant types).Consider using Handlebars
#eachhelper to support multiple values, similar to howauthContextRefsis handled in CreateOIDCClient.hbs (lines 15-20):🔎 View suggested refactor
"redirectUris": [ - "{{redirectUris}}" + {{#each redirectUris}} + "{{this}}" + {{#unless @last}},{{/unless}} + {{/each}} ], "grantTypes": [ - "{{grantTypes}}" + {{#each grantTypes}} + "{{this}}" + {{#unless @last}},{{/unless}} + {{/each}} ], "clientAuthMethods": [ - "{{clientAuthMethods}}" + {{#each clientAuthMethods}} + "{{this}}" + {{#unless @last}},{{/unless}} + {{/each}} ],ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClientResult.hbs (1)
1-5: Duplicate template content.This template is identical to
ui-test/src/main/resources/esignetUI/OidcClient/OIDCClientResult.hbs. Consider consolidating to a single shared template to reduce duplication and maintenance overhead.ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClient.yml (1)
18-22: Fixed userClaims array size limits flexibility.The YAML defines exactly 5 userClaims (userClaims1-5), and the corresponding template (CreateOIDCClient.hbs lines 8-14) hardcodes this array size. This prevents configuring clients with fewer or more user claims.
Consider refactoring to use a flexible array structure with the
#eachhelper, similar toauthContextRefsat line 23.ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClient.hbs (2)
8-14: Hardcoded userClaims array limits flexibility.The template hardcodes exactly 5 userClaims elements. This prevents clients from being configured with fewer or more user claims, which is a common requirement for OIDC clients with varying scopes.
🔎 Apply this refactor to support variable userClaims:
"userClaims": [ - "{{userClaims1}}", - "{{userClaims2}}", - "{{userClaims3}}", - "{{userClaims4}}", - "{{userClaims5}}" + {{#each userClaims}} + "{{this}}" + {{#unless @last}},{{/unless}} + {{/each}} ],Then update the YAML to pass userClaims as an array:
"userClaims": ["name", "email", "gender", "phone_number", "birthdate"]
22-30: Single-element arrays limit flexibility.Similar to OIDCClient.hbs, the
redirectUris,grantTypes, andclientAuthMethodsarrays contain single string placeholders. Consider applying the#eachpattern (like authContextRefs above) to support multiple values.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (84)
ui-test/Biometric Devices/Face/Stream Image/0.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Finger/Slap/Stream Image/1.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Finger/Slap/Stream Image/2.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Finger/Slap/Stream Image/3.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Iris/Double/Stream Image/1.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Iris/Double/Stream Image/2.jpegis excluded by!**/*.jpegui-test/Biometric Devices/Iris/Double/Stream Image/3.jpegis excluded by!**/*.jpegui-test/resource/Profile/Automatic/Auth/0001Face.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Index.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Index.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Little.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Little.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Middle.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Ring.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Left_Thumb.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Index.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Index.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Little.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Little.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Middle.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Ring.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Auth/0001Right_Thumb.wsq.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Exception_Photo.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Face.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Index.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Little.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Left_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Index.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Little.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Automatic/Registration/0001Right_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Face.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Finger_UKNOWN.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Finger_UKNOWN_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Iris_UNKNOWN.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Index.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Index_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Little.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Little_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Middle_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Ring_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Left_Thumb_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Index.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Index_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Little.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Little_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Middle_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Ring_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Default/Auth/Right_Thumb_wsq.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Exception_Photo.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Face.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Index.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Little.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Left_Thumb.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Index.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Iris.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Little.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Middle.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Ring.isois excluded by!**/*.isoui-test/resource/Profile/Default/Registration/Right_Thumb.isois excluded by!**/*.iso
📒 Files selected for processing (107)
ui-test/Biometric Devices/Face/DeviceDiscovery.json(1 hunks)ui-test/Biometric Devices/Face/DeviceInfo.json(1 hunks)ui-test/Biometric Devices/Face/DigitalId.json(1 hunks)ui-test/Biometric Devices/Finger/Single/DeviceDiscovery.json(1 hunks)ui-test/Biometric Devices/Finger/Single/DeviceInfo.json(1 hunks)ui-test/Biometric Devices/Finger/Single/DigitalId.json(1 hunks)ui-test/Biometric Devices/Finger/Slap/DeviceDiscovery.json(1 hunks)ui-test/Biometric Devices/Finger/Slap/DeviceInfo.json(1 hunks)ui-test/Biometric Devices/Finger/Slap/DigitalId.json(1 hunks)ui-test/Biometric Devices/Iris/Double/DeviceDiscovery.json(1 hunks)ui-test/Biometric Devices/Iris/Double/DeviceInfo.json(1 hunks)ui-test/Biometric Devices/Iris/Double/DigitalId.json(1 hunks)ui-test/Biometric Devices/Iris/Single/DeviceDiscovery.json(1 hunks)ui-test/Biometric Devices/Iris/Single/DeviceInfo.json(1 hunks)ui-test/Biometric Devices/Iris/Single/DigitalId.json(1 hunks)ui-test/Biometric Devices/rootca.cer(1 hunks)ui-test/Biometric Devices/rootcaftm.cer(1 hunks)ui-test/application.properties(1 hunks)ui-test/dependency-reduced-pom.xml(1 hunks)ui-test/pom.xml(3 hunks)ui-test/src/main/java/annotations/NeedsUIN.java(1 hunks)ui-test/src/main/java/annotations/NeedsVID.java(1 hunks)ui-test/src/main/java/base/BasePage.java(3 hunks)ui-test/src/main/java/base/BaseTest.java(6 hunks)ui-test/src/main/java/constants/ESignetConstants.java(1 hunks)ui-test/src/main/java/constants/UiConstants.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenIdForUrlEncoded.java(1 hunks)ui-test/src/main/java/models/Uin.java(1 hunks)ui-test/src/main/java/models/Vid.java(1 hunks)ui-test/src/main/java/pages/LoginOptionsPage.java(1 hunks)ui-test/src/main/java/pages/MultiLanguagePage.java(1 hunks)ui-test/src/main/java/runners/Runner.java(1 hunks)ui-test/src/main/java/stepdefinitions/LoginOptionsStepDefinition.java(1 hunks)ui-test/src/main/java/stepdefinitions/LoginWithInjiStepDefinition.java(1 hunks)ui-test/src/main/java/stepdefinitions/MultiLanguageStepDef.java(1 hunks)ui-test/src/main/java/stepdefinitions/SignUpStepDef.java(1 hunks)ui-test/src/main/java/utils/BaseTestUtil.java(8 hunks)ui-test/src/main/java/utils/EsignetConfigManager.java(1 hunks)ui-test/src/main/java/utils/EsignetUtil.java(1 hunks)ui-test/src/main/java/utils/LanguageUtil.java(1 hunks)ui-test/src/main/java/utils/ResourceBundleLoader.java(1 hunks)ui-test/src/main/java/utils/UINManager.java(1 hunks)ui-test/src/main/java/utils/VIDManager.java(1 hunks)ui-test/src/main/java/utils/WaitUtil.java(1 hunks)ui-test/src/main/resources/config.properties(2 hunks)ui-test/src/main/resources/config/AuthPolicy.json(1 hunks)ui-test/src/main/resources/config/AuthPolicy2.json(1 hunks)ui-test/src/main/resources/config/AuthPolicy3.json(1 hunks)ui-test/src/main/resources/config/AuthPolicy4.json(1 hunks)ui-test/src/main/resources/config/AuthPolicy5.json(1 hunks)ui-test/src/main/resources/config/AuthPolicy6.json(1 hunks)ui-test/src/main/resources/config/Authorization/internalAuthRequest.json(1 hunks)ui-test/src/main/resources/config/Authorization/request.json(1 hunks)ui-test/src/main/resources/config/Kernel.properties(1 hunks)ui-test/src/main/resources/config/application.properties(1 hunks)ui-test/src/main/resources/config/policyGroup.json(1 hunks)ui-test/src/main/resources/config/valueMapping.properties(1 hunks)ui-test/src/main/resources/customize-emailable-report-template.html(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentity/AddIdentity.yml(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentity/addIdentityResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentityMock/AddIdentity.hbs(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentityMock/AddIdentity.yml(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentityMock/AddIdentity2.yml(1 hunks)ui-test/src/main/resources/esignetUI/AddIdentityMock/addIdentityResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClient.hbs(1 hunks)ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClient.yml(1 hunks)ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClientResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/OidcClient/OIDCClient.hbs(1 hunks)ui-test/src/main/resources/esignetUI/OidcClient/OIDCClient.yml(1 hunks)ui-test/src/main/resources/esignetUI/OidcClient/OIDCClientResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/ApproveAPIKey/ApproveAPIKey.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/ApproveAPIKey/approveAPIKey.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/ApproveAPIKey/approveAPIKeyResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/CreatePartner/CreatePartner.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/CreatePartner/createPartner.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/CreatePartner/createPartnerResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicy/DefinePolicy.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicy/definePolicy.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicy/definePolicyResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicyGroup/DefinePolicyGroup.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicyGroup/definePolicyGroup.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicyGroup/definePolicyGroupResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/PublishPolicy.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/publishPolicy.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/publishPolicyMispResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/publishPolicyResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/RequestAPIKey.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/requestAPIKeyResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/RequestAPIKey/requestApiKey.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UpdatePartnerCertificate/UpdatePartnerCertificate.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UpdatePartnerCertificate/updatePartnerCertificate.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UpdatePartnerCertificate/updatePartnerCertificateResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/UploadCert.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/uploadCert.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/uploadCert2.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/uploadCert3.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/uploadCertResult.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/UploadCertificate.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/uploadCertificate.hbs(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/uploadCertificateResult.hbs(1 hunks)ui-test/src/main/resources/featurefiles/MultiLanguage.feature(1 hunks)ui-test/src/main/resources/krb5.conf(1 hunks)
⛔ Files not processed due to max files limit (7)
- ui-test/src/main/resources/log4j.properties
- ui-test/src/main/resources/metadata.xml
- ui-test/src/main/resources/spring.properties
- ui-test/src/main/resources/testCaseSkippedList.txt
- ui-test/src/main/resources/validations.properties
- ui-test/testNgXmlFiles/MasterTestSuite.xml
- ui-test/testNgXmlFiles/esignetPrerequisiteSuite.xml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
📚 Learning: 2025-12-02T11:19:44.545Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
Applied to files:
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.javaui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.javaui-test/src/main/resources/esignetUI/AddIdentity/AddIdentity.yml
📚 Learning: 2025-11-28T06:02:21.845Z
Learnt from: sacrana0
Repo: mosip/esignet PR: 1546
File: oidc-service-impl/src/main/java/io/mosip/esignet/services/OAuthServiceImpl.java:198-198
Timestamp: 2025-11-28T06:02:21.845Z
Learning: In the MOSIP esignet codebase, the properties `pushed_authorization_request_endpoint`, `token_endpoint` (from `mosip.esignet.oauth.key-values`), and `issuer` (from `mosip.esignet.discovery.key-values`) are permanently defined in application default properties and are guaranteed to be non-null. Therefore, null checks for these configuration values in OAuthServiceImpl are not required.
Applied to files:
ui-test/src/main/resources/config/Kernel.propertiesui-test/src/main/resources/config/application.propertiesui-test/src/main/java/constants/UiConstants.javaui-test/src/main/resources/config.propertiesui-test/src/main/java/constants/ESignetConstants.java
📚 Learning: 2025-11-17T05:56:18.589Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1535
File: api-test/src/main/resources/esignet/PAR/OauthParNegativeScenarios/OauthParNegativeScenarios.yml:162-162
Timestamp: 2025-11-17T05:56:18.589Z
Learning: In the esignet PAR (Pushed Authorization Requests) implementation, when an invalid client_id is provided, the error response uses "error": "invalid_request" with "error_description": "invalid_client_id", rather than "error": "invalid_client". The test expectations in OauthParNegativeScenarios.yml reflect this implementation behavior.
Applied to files:
ui-test/src/main/resources/esignetUI/CreateClient/CreateOIDCClient.ymlui-test/src/main/resources/esignetUI/OidcClient/OIDCClient.yml
🧬 Code graph analysis (13)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java (2)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.java (2)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java (1)
ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)
ui-test/src/main/java/utils/VIDManager.java (2)
ui-test/src/main/java/models/Vid.java (1)
Vid(3-32)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenIdForUrlEncoded.java (2)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)
ui-test/src/main/java/utils/UINManager.java (2)
ui-test/src/main/java/models/Uin.java (1)
Uin(3-33)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/utils/ResourceBundleLoader.java (2)
ui-test/src/main/java/utils/LanguageUtil.java (1)
LanguageUtil(17-75)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/pages/MultiLanguagePage.java (1)
ui-test/src/main/java/base/BasePage.java (1)
BasePage(29-178)
ui-test/src/main/java/constants/UiConstants.java (1)
ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/utils/BaseTestUtil.java (1)
ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java (1)
ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)
ui-test/src/main/java/base/BaseTest.java (7)
ui-test/src/main/java/models/Uin.java (1)
Uin(3-33)ui-test/src/main/java/models/Vid.java (1)
Vid(3-32)ui-test/src/main/java/utils/BaseTestUtil.java (1)
BaseTestUtil(36-254)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-813)ui-test/src/main/java/utils/UINManager.java (1)
UINManager(11-46)ui-test/src/main/java/utils/VIDManager.java (1)
VIDManager(9-28)
ui-test/src/main/java/utils/EsignetUtil.java (3)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/constants/UiConstants.java (1)
UiConstants(5-32)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
🪛 Gitleaks (8.30.0)
ui-test/src/main/resources/config/application.properties
[high] 101-101: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
ui-test/src/main/resources/config.properties
[high] 61-61: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 62-62: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 64-64: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 65-65: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
ui-test/src/main/java/constants/UiConstants.java (1)
22-30: Consider adding validation or documentation for signup URL configuration.The URL constants concatenate
SIGNUP_BASE_URLwith endpoint paths. IfsignupUrlis not configured (defaulting to empty string), these URLs may be malformed depending on the endpoint property values. Consider either:
- Adding a startup validation check to ensure
signupUrlis configured- Documenting that
signupUrlis a required configuration propertyui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java (1)
89-116: Consider validating configuration property for password.Line 93 retrieves
passwordForAddIdentityfrom properties without null checking. If the property is not configured,nullwill be passed toreplaceKeywordWithValueat line 110, potentially causing unexpected test behavior. Consider adding validation or a default value to improve test reliability.🔎 Optional improvement
-String passWordStr = properties.getProperty("passwordForAddIdentity"); +String passWordStr = properties.getProperty("passwordForAddIdentity"); +if (passWordStr == null || passWordStr.isEmpty()) { + logger.warn("passwordForAddIdentity not configured, using default"); + passWordStr = "DefaultP@ssw0rd123"; +}
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
ui-test/src/main/java/constants/UiConstants.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java(1 hunks)ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenIdForUrlEncoded.java(1 hunks)ui-test/src/main/java/stepdefinitions/SignUpStepDef.java(1 hunks)ui-test/src/main/java/utils/EsignetConfigManager.java(1 hunks)ui-test/src/main/java/utils/EsignetUtil.java(1 hunks)ui-test/src/main/java/utils/UINManager.java(1 hunks)ui-test/src/main/resources/config.properties(2 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/PublishPolicy.yml(1 hunks)ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/UploadCertificate.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- ui-test/src/main/resources/config.properties
- ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePost.java
- ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenIdForUrlEncoded.java
- ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCertificate/UploadCertificate.yml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
📚 Learning: 2025-11-28T06:02:21.845Z
Learnt from: sacrana0
Repo: mosip/esignet PR: 1546
File: oidc-service-impl/src/main/java/io/mosip/esignet/services/OAuthServiceImpl.java:198-198
Timestamp: 2025-11-28T06:02:21.845Z
Learning: In the MOSIP esignet codebase, the properties `pushed_authorization_request_endpoint`, `token_endpoint` (from `mosip.esignet.oauth.key-values`), and `issuer` (from `mosip.esignet.discovery.key-values`) are permanently defined in application default properties and are guaranteed to be non-null. Therefore, null checks for these configuration values in OAuthServiceImpl are not required.
Applied to files:
ui-test/src/main/java/constants/UiConstants.javaui-test/src/main/java/utils/EsignetConfigManager.javaui-test/src/main/java/utils/EsignetUtil.java
📚 Learning: 2025-12-18T12:11:13.984Z
Learnt from: BhuvanaShreeBS
Repo: mosip/esignet PR: 1584
File: ui-test/src/main/java/constants/UiConstants.java:29-30
Timestamp: 2025-12-18T12:11:13.984Z
Learning: In the MOSIP esignet codebase, EsignetConfigManager has two intentionally distinct methods: getproperty(String key) which takes a single parameter, and getProperty(String key, String defaultValue) which takes two parameters. The lowercase 'p' in getproperty is intentional, not a case-sensitivity issue.
Applied to files:
ui-test/src/main/java/constants/UiConstants.javaui-test/src/main/java/utils/EsignetConfigManager.java
📚 Learning: 2025-12-02T11:19:44.545Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
Applied to files:
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java
🧬 Code graph analysis (6)
ui-test/src/main/java/constants/UiConstants.java (1)
ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java (2)
ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-806)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java (3)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-806)
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java (2)
ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)ui-test/src/main/java/utils/EsignetUtil.java (1)
EsignetUtil(50-806)
ui-test/src/main/java/utils/UINManager.java (2)
ui-test/src/main/java/models/Uin.java (1)
Uin(3-33)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/utils/EsignetUtil.java (3)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/constants/UiConstants.java (1)
UiConstants(5-32)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
🔇 Additional comments (8)
ui-test/src/main/java/stepdefinitions/SignUpStepDef.java (1)
14-17: LGTM!The variable shadowing issue has been properly fixed.
this.driveris now correctly assigned frombaseTest.getDriver(), andsignUpPageis initialized with the instance field reference. This ensures the WebDriver is properly shared across the step definition methods.ui-test/src/main/resources/esignetUI/PmsIntegration/PublishPolicy/PublishPolicy.yml (2)
8-14: No changes needed. The presence of bothinputTemplateand inlineinputJSON is the standard and expected pattern in the MOSIP Test Rig framework. The framework uses the template as the structure and substitutes the inline input data into it, rather than treating them as conflicting definitions.
9-21: Both outputTemplate and inline output are intentionally used by the framework.The concern about dual definition is unfounded. The framework method
getJsonFromTemplate(testCaseDTO.getOutput(), testCaseDTO.getOutputTemplate())accepts and uses both parameters together, as confirmed across all test scripts. This is the intended design pattern—outputTemplate points to an external template file while output provides the inline expected values for validation.Regarding boolean values as strings: All test cases across both ui-test and api-test consistently define boolean fields with quoted string values (e.g.,
"true"instead oftrue). This pattern appears intentional and repeated uniformly throughout the test suite, but verify with the framework implementation to ensure this matches the actual API response format.ui-test/src/main/java/utils/EsignetConfigManager.java (1)
65-67: LGTM!The
getSignupUrl()method follows the established pattern in this class and provides a sensible default value (empty string) when the property is not configured.ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java (1)
84-130: LGTM - Well-structured data-driven test implementation.The test method properly handles both ESIGNET and non-ESIGNET execution paths with appropriate input templating, keyword substitution, and output validation. The branching logic for status-code vs. JSON validation is clean and the error handling is adequate.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java (1)
77-107: LGTM - Consistent test implementation.The test method follows the established pattern for data-driven tests in this codebase, with proper input handling, request execution, and output validation.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java (1)
201-218: Good practice restoring interrupt status.The
waittime()method correctly restores the thread's interrupt status in the catch block (line 215), which is important for proper thread management in TestNG.ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java (1)
77-108: LGTM - Follows established test pattern.This test script implementation is consistent with other data-driven test scripts in the PR and properly handles test execution and validation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
ui-test/src/main/java/utils/UINManager.java (3)
18-23: Consider trimming individual UIN values after split.After splitting the comma-separated string, individual UIN values may contain leading/trailing whitespace. Consider applying
trim()to each value before creatingUinobjects to avoid creating UINs with whitespace-only strings.🔎 Suggested improvement
if (uins != null && !uins.isEmpty()) { String[] uinList = uins.split(","); for (String uin : uinList) { - availableUINs.add(new Uin(uin)); + String trimmedUin = uin.trim(); + if (!trimmedUin.isEmpty()) { + availableUINs.add(new Uin(trimmedUin)); + } } }
44-46: Add null validation in releaseUIN().The method doesn't validate that the
uinparameter is non-null before adding it back to the pool. If a null UIN is returned, subsequentacquireUIN()calls will retrieve it, causing NPEs in downstream code.🔎 Suggested improvement
public static void releaseUIN(Uin uin) { + if (uin == null) { + logger.warn("Attempted to release null UIN, ignoring"); + return; + } availableUINs.offer(uin); }
48-50: Document the null return value for timeout expiry.The method can return
nullif the timeout expires before a UIN becomes available. Consider adding JavaDoc or an inline comment to make this explicit for callers.🔎 Suggested improvement
+/** + * Attempts to acquire a UIN with a timeout. + * + * @param timeoutSeconds maximum time to wait in seconds + * @return a UIN if available within the timeout, or null if timeout expires + * @throws InterruptedException if interrupted while waiting + */ public static Uin acquireUINWithTimeout(long timeoutSeconds) throws InterruptedException { return availableUINs.poll(timeoutSeconds, TimeUnit.SECONDS); }ui-test/src/main/java/utils/EsignetUtil.java (6)
52-68: Consider thread safety for mutable static fields.Multiple static fields (
pluginName,signupActiveProfiles,partnerCookie,mobileAuthCookie,triggerESignetKeyGenForPAR,oidc_JWK_Key_For_PAR) are mutable and accessed from multiple methods. If tests run concurrently, these shared static fields may cause race conditions or state corruption.Consider using thread-local storage or synchronization if concurrent test execution is supported.
294-301: Optimize Random instantiation.Creating new
Random()instances multiple times (lines 294, 297, 298, 300) is inefficient. Consider using a single staticRandominstance orThreadLocalRandom.current()for better performance, especially if these methods are called frequently.🔎 Suggested improvement
+private static final Random RANDOM = new Random(); + public static String generateMobileNumberFromRegex() { // ... validation code ... int min = Integer.parseInt(parts[0].trim()); int max = Integer.parseInt(parts[1].trim()); - int length = min + new Random().nextInt(max - min + 1); + int length = min + RANDOM.nextInt(max - min + 1); StringBuilder number = new StringBuilder(); - number.append(new Random().nextInt(9) + 1); + number.append(RANDOM.nextInt(9) + 1); for (int i = 1; i < length; i++) { - number.append(new Random().nextInt(10)); + number.append(RANDOM.nextInt(10)); }
488-499: Add synchronization to RegisteredDetails if accessed concurrently.The
registeredMobileNumberstatic field is mutable and accessed via getter/setter methods without synchronization. If multiple tests run concurrently and share this state, race conditions may occur.Consider making the field thread-local or adding synchronization to the accessor methods.
438-450: Add error handling for parseInt in extractLength().Lines 446-447 call
Integer.parseInt()without error handling. If the regex contains non-numeric values within the braces (e.g.,{a,b}), this will throwNumberFormatExceptionand bypass the fallback logic.🔎 Suggested improvement
private static int extractLength(String regex, boolean isMax) { if (regex == null) return isMax ? 10 : 2; int start = regex.indexOf('{'); int end = regex.indexOf('}'); if (start != -1 && end != -1) { - String[] parts = regex.substring(start + 1, end).split(","); - if (parts.length == 2) - return Integer.parseInt(parts[isMax ? 1 : 0].trim()); - return Integer.parseInt(parts[0].trim()); + try { + String[] parts = regex.substring(start + 1, end).split(","); + if (parts.length == 2) + return Integer.parseInt(parts[isMax ? 1 : 0].trim()); + return Integer.parseInt(parts[0].trim()); + } catch (NumberFormatException e) { + logger.warn("Invalid numeric format in regex: " + regex + ", using default"); + } } return isMax ? 10 : 2; }
521-583: Consider refactoring inputstringKeyWordHandler for maintainability.This method handles multiple keyword replacements and has grown to 60+ lines with multiple responsibilities. Consider extracting each keyword handler (e.g.,
handleClientAssertionPar,handleOidcJwkKey,handleTimestamp) into separate private methods for improved readability and testability.
658-696: Method name doesn't match its behavior.The method name
isTestCaseValidForExecutionsuggests it returns a boolean, but it actually returns aString(the test case name) and throwsSkipExceptionfor invalid cases. Consider renaming tovalidateAndGetTestCaseName()or similar to better reflect its behavior and side effects.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ui-test/src/main/java/utils/EsignetUtil.java(1 hunks)ui-test/src/main/java/utils/UINManager.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-28T06:02:21.845Z
Learnt from: sacrana0
Repo: mosip/esignet PR: 1546
File: oidc-service-impl/src/main/java/io/mosip/esignet/services/OAuthServiceImpl.java:198-198
Timestamp: 2025-11-28T06:02:21.845Z
Learning: In the MOSIP esignet codebase, the properties `pushed_authorization_request_endpoint`, `token_endpoint` (from `mosip.esignet.oauth.key-values`), and `issuer` (from `mosip.esignet.discovery.key-values`) are permanently defined in application default properties and are guaranteed to be non-null. Therefore, null checks for these configuration values in OAuthServiceImpl are not required.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
🧬 Code graph analysis (2)
ui-test/src/main/java/utils/UINManager.java (2)
ui-test/src/main/java/models/Uin.java (1)
Uin(3-33)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
ui-test/src/main/java/utils/EsignetUtil.java (3)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/constants/UiConstants.java (1)
UiConstants(5-32)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
🔇 Additional comments (2)
ui-test/src/main/java/utils/EsignetUtil.java (1)
633-633: Migrate from deprecateddriver.manage().logs()API.The
driver.manage().logs()API is deprecated in Selenium 4.0. For retrieving performance logs specifically, usedevTools.send(Performance.getMetrics)instead. This is the recommended approach for accessing performance metrics.ui-test/src/main/java/utils/UINManager.java (1)
16-16: No action required. The methodgetproperty()with a single parameter is intentionally distinct fromgetProperty()which accepts a default value parameter. Both methods are inherited/defined by design and used throughout the codebase for different purposes.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
ui-test/src/main/java/utils/EsignetUtil.java (7)
53-68: Static mutable state may cause test isolation issues.Multiple static mutable fields (
pluginName,signupActiveProfiles,partnerCookie,mobileAuthCookie,triggerESignetKeyGenForPAR,oidc_JWK_Key_For_PAR) are shared across tests. This can cause:
- Test pollution if tests run in parallel
- Flaky behavior when test execution order changes
Consider adding a
reset()method for test teardown, or document that tests must run sequentially.
88-108: Consider adding a driver cleanup method to prevent resource leaks.
startDriverWithNetwork()creates a ChromeDriver but there's no corresponding cleanup method to close the DevTools session and quit the driver. This could lead to orphaned browser processes if tests don't explicitly handle cleanup.🔎 Suggested addition
public static void stopDriverWithNetwork() { if (devTools != null) { devTools.close(); devTools = null; } if (driver != null) { driver.quit(); driver = null; } lastResponse = null; }
223-228: Consider reducing log verbosity for properties.Line 223 logs the entire properties object (
eachJson.getJSONObject(GlobalConstants.PROPERTIES)), which could be verbose and potentially expose sensitive configuration values in test logs. Consider logging only at DEBUG level or limiting what's logged.🔎 Suggested fix
- logger.info("Found properties: " + eachJson.getJSONObject(GlobalConstants.PROPERTIES)); + if (EsignetConfigManager.IsDebugEnabled()) { + logger.info("Found properties section: " + eachJson.get("name")); + }
292-304: WrapparseIntin try-catch for robustness.Lines 292-293 call
Integer.parseInt()on regex-extracted strings without error handling. If the regex contains malformed content like{a,b}instead of{10,13}, this will throwNumberFormatException. Additionally, creating multipleRandominstances is slightly inefficient.🔎 Suggested fix
+ int min, max; + try { - int min = Integer.parseInt(parts[0].trim()); - int max = Integer.parseInt(parts[1].trim()); + min = Integer.parseInt(parts[0].trim()); + max = Integer.parseInt(parts[1].trim()); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid numeric range in regex: " + digitRange, e); + } + + Random random = new Random(); int length = min + new Random().nextInt(max - min + 1); StringBuilder number = new StringBuilder(); - number.append(new Random().nextInt(9) + 1); + number.append(random.nextInt(9) + 1); for (int i = 1; i < length; i++) { - number.append(new Random().nextInt(10)); + number.append(random.nextInt(10)); }
334-346: Generated passwords have a predictable pattern.The password always starts with uppercase, lowercase, digit, special in that exact order. While functional for tests, this might not properly test password validation edge cases. Consider shuffling the characters.
🔎 Suggested enhancement
+ // Shuffle the password characters for more realistic testing + java.util.List<Character> chars = new java.util.ArrayList<>(); + for (char c : password.toString().toCharArray()) { + chars.add(c); + } + java.util.Collections.shuffle(chars, new Random()); + StringBuilder shuffled = new StringBuilder(); + for (char c : chars) { + shuffled.append(c); + } - return password.toString(); + return shuffled.toString();
350-363: Failed UI spec fetch cannot be retried.If the initial fetch fails (line 357-359),
signupUISpecResponseis set to an emptyJSONObject. Subsequent calls will return this empty object instead of retrying. For transient network issues, this means the entire test run uses an empty spec.🔎 Suggested fix - allow null to trigger retry
private static JSONObject getSignupUISpecResponse() { if (signupUISpecResponse == null) { try { logger.info("Loading Signup UI Spec from " + UiConstants.SIGNUP_UI_SPEC_URL); Response response = RestClient.getRequest(UiConstants.SIGNUP_UI_SPEC_URL, MediaType.APPLICATION_JSON, MediaType.APPLICATION_JSON); signupUISpecResponse = new JSONObject(response.getBody().asString()); } catch (Exception e) { logger.error("Failed to load Signup UI Spec from URL.", e); - signupUISpecResponse = new JSONObject(); + // Return empty object but don't cache it - allows retry on next call + return new JSONObject(); } } return signupUISpecResponse; }
670-677: Simplify boolean comparisons.Lines 670 and 675 use redundant
== truecomparisons. This is a minor style issue but can be simplified.🔎 Suggested fix
- if (isCaptchaEnabled() == true) { + if (isCaptchaEnabled()) { GlobalMethods.reportCaptchaStatus(GlobalConstants.CAPTCHA_ENABLED, true); throw new SkipException(GlobalConstants.CAPTCHA_ENABLED_MESSAGE); } - if (Runner.skipAll == true) { + if (Runner.skipAll) { throw new SkipException(GlobalConstants.PRE_REQUISITE_FAILED_MESSAGE); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui-test/src/main/java/utils/EsignetUtil.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-28T06:02:21.845Z
Learnt from: sacrana0
Repo: mosip/esignet PR: 1546
File: oidc-service-impl/src/main/java/io/mosip/esignet/services/OAuthServiceImpl.java:198-198
Timestamp: 2025-11-28T06:02:21.845Z
Learning: In the MOSIP esignet codebase, the properties `pushed_authorization_request_endpoint`, `token_endpoint` (from `mosip.esignet.oauth.key-values`), and `issuer` (from `mosip.esignet.discovery.key-values`) are permanently defined in application default properties and are guaranteed to be non-null. Therefore, null checks for these configuration values in OAuthServiceImpl are not required.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
📚 Learning: 2025-12-02T11:19:44.545Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
🧬 Code graph analysis (1)
ui-test/src/main/java/utils/EsignetUtil.java (3)
ui-test/src/main/java/constants/ESignetConstants.java (1)
ESignetConstants(3-13)ui-test/src/main/java/constants/UiConstants.java (1)
UiConstants(5-32)ui-test/src/main/java/utils/EsignetConfigManager.java (1)
EsignetConfigManager(15-69)
🔇 Additional comments (10)
ui-test/src/main/java/utils/EsignetUtil.java (10)
1-50: Imports and class structure look appropriate.The imports are well-organized and align with the functionality provided (Selenium WebDriver, JSON handling, JWT signing, REST client). The class appropriately extends
AdminTestUtilfor shared test utilities.
79-86: LGTM!The helper method appropriately logs errors and returns null, which callers handle correctly.
120-132: LGTM!Simple caching pattern for plugin name retrieval.
419-436: LGTM!The
FullNameclass with public fields is appropriate for a simple test data holder. The name generation logic correctly handles edge cases withMath.max()and trims trailing spaces.
488-499: LGTM!The
RegisteredDetailsnested class provides a clean way to share test context (mobile number) across test flows.
585-596: LGTM!The fix from the previous review was correctly applied. The method now properly handles exceptions and returns null on failure.
632-660: LGTM!The null check for
driverwas correctly added. The exception handling in the log parsing loop is appropriate since performance logs may contain malformed entries.
702-740: LGTM!Token retrieval with caching and validity checking is well implemented. The use of
optStringwith defaults prevents NPE.
742-764: LGTM!The method handles authentication edge cases (invalid token, no auth) appropriately and includes proper request/response reporting for test debugging.
509-519: No issues found. The code correctly checks and retrieves the"esignetSupportedLanguage"property using the same constant throughout.ESignetConstants.ESIGNET_SUPPORTED_LANGUAGEis defined as"esignetSupportedLanguage", not"ESignetUI_"(which is a separate constantESIGNET_STRING).Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ui-test/src/main/java/utils/EsignetUtil.java (1)
438-458: Consider adding error handling for malformed digit ranges in extractLength.Lines 446-447 call
Integer.parseInt()without try-catch. If the regex contains braces with non-numeric content (e.g.,{abc,def}), this will throwNumberFormatException. While the method handles null regex by returning defaults, it doesn't guard against malformed numeric values.🔎 Optional defensive wrapper
private static int extractLength(String regex, boolean isMax) { if (regex == null) return isMax ? 10 : 2; int start = regex.indexOf('{'); int end = regex.indexOf('}'); if (start != -1 && end != -1) { String[] parts = regex.substring(start + 1, end).split(","); - if (parts.length == 2) - return Integer.parseInt(parts[isMax ? 1 : 0].trim()); - return Integer.parseInt(parts[0].trim()); + try { + if (parts.length == 2) + return Integer.parseInt(parts[isMax ? 1 : 0].trim()); + return Integer.parseInt(parts[0].trim()); + } catch (NumberFormatException e) { + logger.warn("Invalid numeric range in regex: " + regex + ". Using default."); + return isMax ? 10 : 2; + } } return isMax ? 10 : 2; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui-test/src/main/java/utils/EsignetUtil.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-28T06:02:21.845Z
Learnt from: sacrana0
Repo: mosip/esignet PR: 1546
File: oidc-service-impl/src/main/java/io/mosip/esignet/services/OAuthServiceImpl.java:198-198
Timestamp: 2025-11-28T06:02:21.845Z
Learning: In the MOSIP esignet codebase, the properties `pushed_authorization_request_endpoint`, `token_endpoint` (from `mosip.esignet.oauth.key-values`), and `issuer` (from `mosip.esignet.discovery.key-values`) are permanently defined in application default properties and are guaranteed to be non-null. Therefore, null checks for these configuration values in OAuthServiceImpl are not required.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
📚 Learning: 2025-11-17T05:56:18.589Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1535
File: api-test/src/main/resources/esignet/PAR/OauthParNegativeScenarios/OauthParNegativeScenarios.yml:162-162
Timestamp: 2025-11-17T05:56:18.589Z
Learning: In the esignet PAR (Pushed Authorization Requests) implementation, when an invalid client_id is provided, the error response uses "error": "invalid_request" with "error_description": "invalid_client_id", rather than "error": "invalid_client". The test expectations in OauthParNegativeScenarios.yml reflect this implementation behavior.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
📚 Learning: 2025-12-02T11:19:44.545Z
Learnt from: prathmeshj12
Repo: mosip/esignet PR: 1560
File: api-test/src/main/resources/esignet/LinkedConsent/LinkedAuthenticationConsentWla/LinkedAuthenticationConsentWla.yml:512-566
Timestamp: 2025-12-02T11:19:44.545Z
Learning: In the esignet test suite file LinkedAuthenticationConsentWla.yml, negative test cases can include successful sub-operation responses (sendOtpResp and validateOtpResp) alongside the expected error response. This composite structure validates that OTP operations can succeed while the main authentication fails, allowing comprehensive testing of the authentication flow.
Applied to files:
ui-test/src/main/java/utils/EsignetUtil.java
🔇 Additional comments (6)
ui-test/src/main/java/utils/EsignetUtil.java (6)
270-304: Previous validation concerns properly addressed.The null checks and format validation for the regex parsing were correctly implemented. The method now safely handles missing configuration, malformed regex patterns, and invalid digit ranges with clear error messages.
535-564: RSA key parsing error handling correctly implemented.The
RuntimeExceptionis now properly thrown at line 543 when RSA key parsing fails, preventing the null key from being passed tosignJWKKey(). This addresses the previous NPE concern.
599-631: Time calculation fix confirmed.Line 613 now correctly uses
calendar.add(Calendar.SECOND, idTokenExpirySecs)instead of integer division by 3600, which properly handles sub-hour expiry values.
633-661: Driver null check properly added.The null check at lines 634-637 prevents
NullPointerExceptionwhengetOauthDetailsBody()is called before driver initialization, with a clear error message logged.
796-834: PAR response validation correctly implemented.Lines 822-831 now include proper validation for null response and missing
request_urifield, with clear error messages and exceptions thrown. This prevents silent failures in the PAR flow.
1-836: Well-structured utility class with comprehensive test automation support.The implementation provides robust support for Esignet UI test automation with good separation of concerns:
- Network capture using Chrome DevTools for request verification
- Comprehensive actuator config access with intelligent fallback logic
- PAR/OAuth workflow support with proper JWT signing
- Dynamic test data generation from configuration
- UI spec parsing and field validation
- Proper caching strategies to minimize redundant calls
All previously flagged critical issues have been properly addressed. The code demonstrates good error handling practices and extensive logging.
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
…lient Setup Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
BhuvanaShreeBS
force-pushed
the
develop
branch
from
c7897a4 to
e703d21
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@ui-test/application.properties`:
- Around line 32-38: Remove hard-coded secrets from application.properties:
replace plaintext entries such as mosip.mock.sbi.file.face.keys.keystorepwd and
mosip.mock.sbi.file.face.keys.keystorepwd.ftm (and any mosip.auth.secretkey
occurrences) with configuration lookups that read from environment variables or
a secret manager; update the code that reads these properties to fallback to
process.env/SecretManager calls (or a config loader) so passwords/keys are
injected at runtime rather than stored in the repo, and ensure all other secret
property keys pointed out in the review (e.g., lines referenced 46-52, 60-66,
73-76) follow the same pattern.
In `@ui-test/Biometric` Devices/Finger/Single/DeviceInfo.json:
- Line 12: The deviceCode for the Finger/Single fixture is incorrect (it uses
"slap"); update the "deviceCode" value in the Finger/Single fixture JSON (the
"deviceCode" field currently set to "b692b595-3523-slap-99fc-bd76e35f290f") to
use the correct identifier for this fixture (e.g., replace "slap" with "finger"
or "single" so the UUID encodes the Finger/Single type) ensuring the new string
follows the same UUID-like pattern as other fixtures.
In `@ui-test/Biometric` Devices/rootcaftm.cer:
- Around line 1-21: The file rootcaftm.cer contains an expired test certificate
(valid 2021-06-10 to 2022-06-10) and appears unused; either delete the
rootcaftm.cer file to remove this orphaned fixture or, if it must be kept, add a
short documented rationale (e.g., in the test fixtures README or a header
comment in the file) explaining its purpose and why an expired cert is required,
and ensure there are no references to rootcaftm.cer in tests or CI.
In `@ui-test/src/main/java/base/BaseTest.java`:
- Around line 114-124: The thread-local UIN/VID set in handleUIN and handleVID
can leak between scenarios; add an `@After` hook (e.g., a method like
cleanupUinVid) that checks threadUin and threadVid, calls the appropriate
release methods on the managers (UINManager.releaseUIN(threadUin.get()) and
VIDManager.releaseVID(threadVid.get()) if available), and then calls
threadUin.remove() and threadVid.remove() to clear the ThreadLocals so no stale
IDs remain on the thread after each scenario.
In
`@ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java`:
- Around line 124-144: The UIN fetched via
JsonPrecondtion.getValueFromJson(RestClient.getRequestWithCookie(...)) can be
null/empty and will cause NPEs at inputJson.replace; after retrieving String
uin, add a guard that checks for null or blank and if so throw a clear
AdminTestException (or equivalent) with a descriptive message including the role
and the raw response/context (ApplnURI call or token). Ensure the check is
placed before any use of uin (before inputJson.replace) and reference
JsonPrecondtion.getValueFromJson, RestClient.getRequestWithCookie, and
inputJson.replace when implementing the guard.
In
`@ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java`:
- Around line 114-127: The current setResultTestName method uses TestNG
internals (TestResult, BaseTestMethod, fields m_method/m_methodName) via
reflection; replace this by implementing TestNG's public ITest interface on the
test class and return the dynamic name (testCaseName) from getTestName (set
testCaseName in the test constructor or `@BeforeMethod`), or if refactoring all
files now is infeasible, move the reflection logic into a single shared utility
method (e.g., TestNameUtils.renameTestResult) with clear documentation about the
TestNG versions it supports and a TODO to migrate to ITest; update references to
setResultTestName to use the new approach.
In
`@ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java`:
- Around line 84-129: The ESIGNET branch only assigns response for the "mock"
plugin, leaving response null for other plugins; update the test(TestCaseDTO)
method so that inside the if
(testCaseName.contains(ESignetConstants.ESIGNET_STRING)) block, after the
getPluginName().equals("mock") branch, add an else that either (a) calls the
non-mock request helper (postWithBodyAndCookieForAutoGeneratedId or the
appropriate EsignetUtil.postWithBodyAndBearerToken equivalent) to set the
response variable, or (b) throws a clear AdminTestException indicating the
plugin is unsupported; reference getPluginName(), response,
EsignetUtil.postWithBodyAndBearerToken, postWithBodyAndCookieForAutoGeneratedId,
and writeAutoGeneratedId to locate where to insert the fix.
In `@ui-test/src/main/java/models/Uin.java`:
- Around line 24-30: The Uin.toString() implementation currently returns raw
values for the sensitive fields uin, phone, and email; update the Uin.toString()
method to avoid exposing PII by either omitting these fields or returning masked
values (e.g., show only last N chars or replace with "<redacted>") for uin,
phone, and email, and keep any non-sensitive class identifier text for
readability; ensure you update references/tests that rely on the full string if
necessary.
In `@ui-test/src/main/java/models/Vid.java`:
- Around line 24-30: The Vid.toString() currently returns full PII for fields
vid, phone, and email; update the toString() method in class Vid to avoid
leaking sensitive data by omitting or masking these fields (e.g., replace values
with "REDACTED" or a masked form showing only non-sensitive fragments or
lengths). Locate the toString() method and change its output to include safe
placeholders or masked representations for vid, phone, and email instead of the
raw field values.
In `@ui-test/src/main/java/runners/Runner.java`:
- Around line 63-97: The fallback branch of the scenarios() DataProvider calls
browsers.getFirst() without checking for an empty list; add an explicit check on
browsers.isEmpty() before using getFirst() in the fallback construction, and
handle the empty case (e.g., log and throw a clear IllegalStateException or use
a configured default browser) so scenarios() does not throw
NoSuchElementException at runtime; update the fallback logic that builds
List<Object[]> fallback to use the safe/browser-default value instead of
directly calling browsers.getFirst().
🧹 Nitpick comments (16)
ui-test/src/main/resources/esignetUI/PmsIntegration/DefinePolicy/definePolicy.hbs (1)
10-110: Minor: Inconsistent indentation style.The
policiesblock (lines 10-110) uses tabs for indentation while the rest of the file uses spaces. Consider normalizing to a consistent indentation style throughout the file for better maintainability.ui-test/src/main/resources/config/AuthPolicy6.json (1)
1-41: Minor formatting inconsistencies in JSON.The JSON structure is valid and the policy configuration looks correct. However, there are some formatting inconsistencies:
- Lines 35-36 use tabs for indentation while the rest of the file uses spaces.
- Line 22 has trailing whitespace after the closing brace.
- File is missing a trailing newline.
Consider normalizing the indentation to use consistent spacing throughout.
🧹 Suggested formatting fix
{ - "authSubType": "", - "authType": "otp-request", - "mandatory": false - }, + "authSubType": "", + "authType": "otp-request", + "mandatory": false + }, { "authSubType": "", "authType": "kyc", "mandatory": false }, { "authSubType": "", "authType": "demo", "mandatory": false - } + } ], "kycLanguages": [ - "ara", - "eng" - ], + "ara", + "eng" + ], "allowedKycAttributes": [ ], - "authTokenType": "partner" - } + "authTokenType": "partner" + }ui-test/dependency-reduced-pom.xml (1)
124-129: Minor observation: Empty transformer element.Line 125 contains an empty
<transformer />element in the shade plugin configuration. This appears to be pre-existing but has no effect. If this was intended for a specific transformation (e.g.,ServicesResourceTransformer), it should be configured; otherwise, it can be removed to clean up the configuration.<configuration> <finalName>${fileName}</finalName> <transformers> - <transformer /> <transformer> <mainClass>runners.Runner</mainClass> </transformer>ui-test/src/main/resources/esignetUI/OidcClient/OIDCClient.hbs (2)
1-21: Inconsistent indentation throughout the template.The indentation varies across different sections (2 spaces, 6 spaces, 8 spaces, 12 spaces), which reduces readability. Consider standardizing to consistent 2-space or 4-space indentation.
🧹 Suggested formatting fix
{ "request": { "name": "{{name}}", "policyId": "{{policyId}}", "publicKey": {{publicKey}}, "authPartnerId": "{{authPartnerId}}", "logoUri": "{{logoUri}}", "redirectUris": [ - "{{redirectUris}}" - ], + "{{redirectUris}}" + ], "grantTypes": [ - "{{grantTypes}}" - ], + "{{grantTypes}}" + ], "clientAuthMethods": [ - "{{clientAuthMethods}}" - ], - "additionalConfig": { - "require_pushed_authorization_requests": {{require_pushed_authorization_requests}} - } + "{{clientAuthMethods}}" + ], + "additionalConfig": { + "require_pushed_authorization_requests": {{require_pushed_authorization_requests}} + } } }
8-16: Array template approach correctly supports current test scenarios.The template generates single-element JSON arrays (
["authorization_code"],["private_key_jwt"]), which matches the test data being passed (single string values). This is valid OIDC configuration.If future test scenarios require multiple redirect URIs, grant types, or client authentication methods, consider refactoring to use Handlebars iteration:
ui-test/src/main/resources/esignetUI/PmsIntegration/UploadCert/uploadCert2.hbs (1)
2-10: ParameterizepartnerId(and optionally the certificate) for reuse across environments.Hard-coding
"Tech-123"and the PEM makes this template brittle when running against different partners or environments. Consider templating these values to reduce future duplication and test maintenance.ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java (2)
35-37: Field visibility inconsistency and potential thread-safety issue withpathParams.The
pathParamsfield has package-private visibility (no modifier) whiletestCaseNameisprotectedandresponseispublic. More importantly,pathParamsis set in the@DataProvidermethod but used in the@Testmethod. If tests run in parallel with different XML configurations, this could lead to race conditions wherepathParamsgets overwritten.Consider making
pathParamsconsistent with the pattern and potentially passing it through theTestCaseDTOinstead.♻️ Suggested visibility fix
public class PutWithPathParamsAndBody extends EsignetUtil implements ITest { private static final Logger logger = Logger.getLogger(PutWithPathParamsAndBody.class); protected String testCaseName = ""; - String pathParams = null; + protected String pathParams = null; public Response response = null;
68-76: Outdated Javadoc comment.The Javadoc references "OTP Generation execution" with parameters
objTestParameters,testScenario, andtestcaseNamethat don't match the actual method signature which only takesTestCaseDTO testCaseDTO.📝 Suggested Javadoc fix
/** - * Test method for OTP Generation execution + * Test method for PUT request with path parameters and body * - * `@param` objTestParameters - * `@param` testScenario - * `@param` testcaseName + * `@param` testCaseDTO the test case data transfer object * `@throws` AuthenticationTestException * `@throws` AdminTestException + * `@throws` SecurityXSSException */ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PostWithOnlyPathParam.java (3)
33-128: Significant code duplication withPutWithPathParamsAndBodyand other test scripts.This class shares approximately 90% of its structure with
PutWithPathParamsAndBody.javaand other test scripts in the same package. The only differences are:
- The HTTP method used (POST vs PUT)
- The specific request method called (
postWithOnlyPathParamAndCookievsputWithPathParamsBodyAndCookie)- Configuration parameters (
sendEsignetTokenvspathParams)Consider extracting the common logic into a base class or template method pattern to reduce maintenance burden and ensure consistent behavior across test scripts.
110-113: Typo in Javadoc comment.Line 111 contains a typo: "ser" should be "set".
📝 Fix typo
/** - * The method ser current test name to result + * The method sets current test name to result * * `@param` result */
68-76: Outdated Javadoc comment (same issue as PutWithPathParamsAndBody).The Javadoc references "OTP Generation execution" with incorrect parameter names that don't match the actual method signature.
ui-test/src/main/java/utils/EsignetUtil.java (5)
323-346: Password generation uses multipleRandominstances instead of reusing one.The
generateValidPasswordFromActuatormethod creates a newRandom()instance on each call tonextInt(). This is inefficient and can produce poor randomness if called rapidly. Consider creating a singleRandominstance and reusing it.♻️ Suggested optimization
public static String generateValidPasswordFromActuator() { int min = getPasswordMinLength(); int max = getPasswordMaxLength(); - int length = min + new Random().nextInt(max - min + 1); + Random random = new Random(); + int length = min + random.nextInt(max - min + 1); String upper = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; String lower = "abcdefghijklmnopqrstuvwxyz"; String digits = "0123456789"; String special = "_!@#$%^&*"; String all = upper + lower + digits + special; StringBuilder password = new StringBuilder(); - password.append(upper.charAt(new Random().nextInt(upper.length()))); - password.append(lower.charAt(new Random().nextInt(lower.length()))); - password.append(digits.charAt(new Random().nextInt(digits.length()))); - password.append(special.charAt(new Random().nextInt(special.length()))); + password.append(upper.charAt(random.nextInt(upper.length()))); + password.append(lower.charAt(random.nextInt(lower.length()))); + password.append(digits.charAt(random.nextInt(digits.length()))); + password.append(special.charAt(random.nextInt(special.length()))); for (int i = 4; i < length; i++) { - password.append(all.charAt(new Random().nextInt(all.length()))); + password.append(all.charAt(random.nextInt(all.length()))); } return password.toString(); }
460-486: SameRandominstance inefficiency in name generation methods.Both
generateEnglishNameandgenerateKhmerNamecreate a singleRandominstance per method butgenerateMobileNumberFromRegex(lines 296-304) creates multiple instances. Consider standardizing on a single sharedRandominstance or creating one per method consistently.
671-698: Redundant boolean comparisons reduce readability.The code uses explicit
== trueand== falsecomparisons which are redundant and can be simplified.♻️ Suggested simplification
- if (isCaptchaEnabled() == true) { + if (isCaptchaEnabled()) { GlobalMethods.reportCaptchaStatus(GlobalConstants.CAPTCHA_ENABLED, true); throw new SkipException(GlobalConstants.CAPTCHA_ENABLED_MESSAGE); } - if (Runner.skipAll == true) { + if (Runner.skipAll) { throw new SkipException(GlobalConstants.PRE_REQUISITE_FAILED_MESSAGE); } if (pluginName.equals("mock")) { BaseTestCase.setSupportedIdTypes(Arrays.asList("UIN")); String endpoint = testCaseDTO.getEndPoint(); - if (endpoint.contains("/esignet/") == false && endpoint.contains("/mock-identity-system/") == false) { + if (!endpoint.contains("/esignet/") && !endpoint.contains("/mock-identity-system/")) { throw new SkipException(GlobalConstants.FEATURE_NOT_SUPPORTED_MESSAGE); } } else if (pluginName.equals("mosipid")) { getSupportedIdTypesValueFromActuator(); logger.info("supportedIdType = " + supportedIdType); String endpoint = testCaseDTO.getEndPoint(); - if (endpoint.contains("/mock-identity-system/") == true + if (endpoint.contains("/mock-identity-system/") || ((testCaseName.equals("ESignetUI_CreateOIDCClient_all_Valid_Smoke_sid")) - && endpoint.contains("/v1/esignet/client-mgmt/client"))) { + && endpoint.contains("/v1/esignet/client-mgmt/client"))) { throw new SkipException(GlobalConstants.FEATURE_NOT_SUPPORTED_MESSAGE); } }
720-741: Potential race condition in token caching.The
getAuthTokenByRolemethod checks and sets static fields (partnerCookie,mobileAuthCookie) without synchronization. If multiple tests run concurrently with different roles, this could lead to race conditions where:
- Thread A checks
isValidToken(partnerCookie)returns false- Thread B checks
isValidToken(partnerCookie)returns false- Both threads call
getAuthTokenFromKeyCloaksimultaneously- Wasted API calls and potential token confusion
For a test utility this may be acceptable, but consider using
synchronizedorvolatileif parallel test execution is expected.
488-499: Thread-safety concern withRegisteredDetailsstatic state.The nested
RegisteredDetailsclass uses a static fieldregisteredMobileNumberthat can be accessed and modified by any test. If tests run in parallel and share this state, one test could overwrite another test's mobile number.Consider whether this shared state is intentional for the test workflow, or if it should be thread-local or passed explicitly.
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/AddIdentity.java
Show resolved
Hide resolved
...st/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/PutWithPathParamsAndBody.java
Show resolved
Hide resolved
ui-test/src/main/java/io/mosip/testrig/apirig/esignetUI/testscripts/SimplePostForAutoGenId.java
Show resolved
Hide resolved
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
2 participants
Integrated API Support in Esignet UI Automation for OIDC Client Creation
Summary by CodeRabbit
New Features
Tests
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.