Skip to content

ES-2914 | updated url#1674

Merged
ase-101 merged 1 commit intomosip:developfrom
nandhu-kumar:develop-local
Mar 18, 2026
Merged

ES-2914 | updated url#1674
ase-101 merged 1 commit intomosip:developfrom
nandhu-kumar:develop-local

Conversation

@nandhu-kumar
Copy link
Contributor

@nandhu-kumar nandhu-kumar commented Mar 17, 2026

Summary by CodeRabbit

Release Notes

  • Chores
    • Made captcha validation endpoint configurable via environment variables, replacing previously hardcoded values. This provides greater deployment flexibility.

Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Walkthrough

The change introduces a new configurable property mosip.esignet.captcha.url sourced from an environment variable, replacing the hardcoded captcha validator endpoint URL with a dynamic reference to this property. This externalizes the captcha service configuration.

Changes

Cohort / File(s) Summary
Configuration Properties
esignet-service/src/main/resources/application-default.properties
Added new mosip.esignet.captcha.url property with environment variable MOSIP_API_INTERNAL_HOST fallback. Updated captcha validator endpoint to use configurable base URL instead of hardcoded http://captcha.captcha.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A URL once fixed now dances free,
Through env variables wild it roams with glee,
Config-ured paths, no more hardcoded stone—
The captcha service finds its home!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and uses generic terminology ('updated url') without clearly identifying which URL or the specific purpose of the change. Consider using a more descriptive title like 'Make captcha URL configurable via environment variable' that clearly indicates what URL is being changed and why.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
esignet-service/src/main/resources/application-default.properties (1)

81-83: Consider documenting URL format requirements.

The validator URL is constructed by concatenating ${mosip.esignet.captcha.url} with /v1/captcha/validatecaptcha. If the MOSIP_API_INTERNAL_HOST environment variable is set with a trailing slash (e.g., http://captcha.captcha/), the resulting URL will contain double slashes: http://captcha.captcha//v1/captcha/validatecaptcha.

While most HTTP clients handle double slashes gracefully, consider adding a comment to document that the base URL should not include a trailing slash, or implement URL normalization in the Java code.

📝 Suggested documentation
 ## captcha validation is enabled for the auth-factors - otp, pwd, bio and pin.
+# Note: MOSIP_API_INTERNAL_HOST should not include a trailing slash
 mosip.esignet.captcha.url=${MOSIP_API_INTERNAL_HOST:http://captcha.captcha}
 mosip.esignet.captcha.required=send-otp,pwd,kbi,binding-otp
 mosip.esignet.captcha.validator-url=${mosip.esignet.captcha.url}/v1/captcha/validatecaptcha
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@esignet-service/src/main/resources/application-default.properties` around
lines 81 - 83, The validator URL construction in the properties uses
mosip.esignet.captcha.url + "/v1/captcha/validatecaptcha" which can produce
double slashes if MOSIP_API_INTERNAL_HOST ends with a trailing slash; update the
properties file to include a comment documenting that mosip.esignet.captcha.url
must not end with a trailing slash (e.g., "Ensure MOSIP_API_INTERNAL_HOST has no
trailing slash"), and/or modify the code that reads mosip.esignet.captcha.url
(where the validator URL is assembled) to normalize the base URL by trimming any
trailing '/' before appending "/v1/captcha/validatecaptcha" (adjust the
normalization in the Java method that builds the validator URL).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@esignet-service/src/main/resources/application-default.properties`:
- Around line 81-83: The validator URL construction in the properties uses
mosip.esignet.captcha.url + "/v1/captcha/validatecaptcha" which can produce
double slashes if MOSIP_API_INTERNAL_HOST ends with a trailing slash; update the
properties file to include a comment documenting that mosip.esignet.captcha.url
must not end with a trailing slash (e.g., "Ensure MOSIP_API_INTERNAL_HOST has no
trailing slash"), and/or modify the code that reads mosip.esignet.captcha.url
(where the validator URL is assembled) to normalize the base URL by trimming any
trailing '/' before appending "/v1/captcha/validatecaptcha" (adjust the
normalization in the Java method that builds the validator URL).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: af35390b-aa47-4f89-9907-e55b1cda0ff6

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa9c2c and d8dea38.

📒 Files selected for processing (1)
  • esignet-service/src/main/resources/application-default.properties

@ase-101 ase-101 merged commit b67bc99 into mosip:develop Mar 18, 2026
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants