Release 3.6.0 #284
Release 3.6.0 #284
Conversation
Fix identity api url
IDENTITY_BASE_URL fix
Fix identity variable
fix IDENTITY_API_BASE_URL variable in docker file
Fix url issue
fix IDENTITY_API_BASE_URL variable in docker file
fix IDENTITY_API_BASE_URL variable in docker file
fix IDENTITY_API_BASE_URL variable in docker file
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (4)
src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java (1)
55-56: Off-by-one day-start: records at 00:00:00 are excludedUsing
"00:00:01"skips calls exactly at midnight.- fromDate = dateArr[0].concat(" 00:00:01"); + fromDate = dateArr[0].concat(" 00:00:00");Optional: compute with java.time to avoid string concat and TZ/DST edge cases.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (3)
140-141: Do not log plaintext passwords.This leaks credentials in logs.
- logger.info("userAuthenticate request - " + m_User + " " + m_User.getUserName() + " " + m_User.getPassword()); + logger.info("userAuthenticate request - userName={}", m_User.getUserName());
166-166: Remove hardcoded AES key; inject from secure config.Hardcoding secrets is a critical security risk.
- String decryptPassword = aesUtil.decrypt("Piramal12Piramal", m_User.getPassword()); + String decryptPassword = aesUtil.decrypt(aesKey, m_User.getPassword());- String decryptPassword = aesUtil.decrypt("Piramal12Piramal", m_User.getPassword()); + String decryptPassword = aesUtil.decrypt(aesKey, m_User.getPassword());- String decryptPassword = aesUtil.decrypt("Piramal12Piramal", m_user.getPassword()); + String decryptPassword = aesUtil.decrypt(aesKey, m_user.getPassword());Add the injected key (store it in Vault/ENV, not in code):
@Value("${security.aes-key}") private String aesKey;Also applies to: 447-447, 672-672
389-391: Stop exposing credentials in API responses.agentPassword is sensitive; do not send to clients.
- resMap.put("agentPassword", mUser.getAgentPassword());- previlegeObj.getJSONObject(serv).put("agentPassword", m_UserServiceRoleMapping.getAgentPassword());Also applies to: 417-418
β»οΈ Duplicate comments (6)
src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java (1)
41-42: Reinstate external toggle; avoid default-on schedulerHardcoding
truemakes this run in all environments. Drive it via property as earlier feedback recommended.Apply:
- // @Value("${start-ctidatacheck-scheduler}") - private boolean startCtiDataCheckFlag=true; + @Value("${start-ctidatacheck-scheduler:false}") + private boolean startCtiDataCheckFlag;Optional (preferable): gate bean creation instead of runtime flagging.
// imports import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @ConditionalOnProperty(value = "start-ctidatacheck-scheduler", havingValue = "true", matchIfMissing = false) @EnableScheduling @Configuration public class NHMDetailCallReportScheduler { // you may keep/remove the if (startCtiDataCheckFlag) guard as a second line of defense }src/main/java/com/iemr/common/utils/JwtUtil.java (1)
148-156: UserId accessor fixed correctlyNow returns the "userId" claim as intended. If downstream expects numeric, consider switching the claim type to Long in a future change.
src/main/resources/application.properties (2)
172-175: Confirm JWT secret is sourced from env across all env files.Per team standard, jwt.secret must not live in repo. Ensure itβs defined only via env (e.g., jwt.secret=${JWT_SECRET:}) and not present in any committed properties.
I can scan the repo and open a follow-up if any stragglers remain.
369-377: Externalize DB creds and fix deprecated MySQL driver.Use env indirection for URLs and credentials; update driver to com.mysql.cj.jdbc.Driver.
Apply:
-spring.datasource.url=jdbc:mysql://localhost:3306/db_iemr -spring.datasource.username=<Enter userName> -spring.datasource.password=<Enter password> +spring.datasource.url=${DB_URL:} +spring.datasource.username=${DB_USERNAME:} +spring.datasource.password=${DB_PASSWORD:} -secondary.datasource.username=<Enter username> -secondary.datasource.password=<Enter password> -secondary.datasource.url=jdbc:mysql://localhost:3306/db_reporting -secondary.datasource.driver-class-name=com.mysql.jdbc.Driver +secondary.datasource.username=${REPORTING_DB_USERNAME:} +secondary.datasource.password=${REPORTING_DB_PASSWORD:} +secondary.datasource.url=${REPORTING_DB_URL:} +secondary.datasource.driver-class-name=com.mysql.cj.jdbc.Driversrc/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
83-85: Provide a safe default for CAPTCHA flag to avoid startup failure.If the property is missing, the app will fail to start. Default to false.
- @Value("${captcha.enable-captcha}") + @Value("${captcha.enable-captcha:false}") private boolean enableCaptcha;
560-585: Validate JWT and handle Bearer prefix; null-safe header check.Prevents NPE and token parsing on unverified input. Also standardize the cookie/header name via constant.
- String authHeader = request.getHeader("Authorization"); - if (authHeader.isEmpty()) { + String authHeader = request.getHeader("Authorization"); + if (authHeader == null || authHeader.isBlank()) { // Try JWT token from header first - String jwtToken = request.getHeader("Jwttoken"); + String jwtToken = request.getHeader(Constants.JWT_TOKEN); // If not in header, try cookie if (jwtToken == null) { Cookie[] cookies = request.getCookies(); if (cookies != null) { for (Cookie cookie : cookies) { - if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { + if (Constants.JWT_TOKEN.equalsIgnoreCase(cookie.getName())) { jwtToken = cookie.getValue(); break; } } } } - if (jwtToken == null) { + if (jwtToken == null || jwtToken.isBlank()) { logger.warn("Authentication failed: no token found in header or cookies."); throw new IEMRException("Authentication failed. Please log in again."); } - // Extract user ID from the JWT token - String userId = jwtUtil.getUserIdFromToken(jwtToken); + // Validate and extract userId + String token = jwtToken.startsWith("Bearer ") ? jwtToken.substring(7) : jwtToken; + Claims claims = jwtUtil.validateToken(token); + if (claims == null) { + throw new IEMRException("Authentication failed. Please log in again."); + } + String userId = claims.get("userId", String.class);
π§Ή Nitpick comments (12)
src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java (3)
65-67: Normalize MSISDN consistently; current check misses several real-world formatsLength-based trim only for
>10misses0-prefixed 10-digits and country codes; also duplicates logic.Replace both places with a single helper:
- if(null != phoneNo && phoneNo.length()>10) { - phoneNo = phoneNo.replaceFirst("^0+", ""); - } + phoneNo = normalizeMsisdn(phoneNo);- String phoneNo = detailedCallReport.getPHONE(); - if(null != phoneNo && phoneNo.length()>10) - phoneNo = phoneNo.replaceFirst("^0+", ""); - beneficiaryCall.setPhoneNo(phoneNo); + String phoneNo = normalizeMsisdn(detailedCallReport.getPHONE()); + beneficiaryCall.setPhoneNo(phoneNo);Add helper inside the class:
private static String normalizeMsisdn(String raw) { if (raw == null) return null; String digits = raw.replaceAll("\\D+", ""); // Keep last 10 digits (drops leading 0s/country code like 91) if (digits.length() > 10) digits = digits.substring(digits.length() - 10); return digits; }Also applies to: 156-159
166-167: Remove duplicate setters
setCallEndTimeandsetCallDurationare set twice; keep the later ones.- beneficiaryCall.setCallEndTime(detailedCallReport.getCallEndTime()); - beneficiaryCall.setCallDuration(String.valueOf(detailedCallReport.getCall_Duration()));
70-73: Mask PII in logs and use parameterized loggingAvoid logging full phone numbers and entire entity dumps.
Example:
private static String maskMsisdn(String msisdn) { if (msisdn == null || msisdn.length() < 4) return "****"; return "******" + msisdn.substring(msisdn.length() - 4); }Then:
logger.info("Record exists for sessionID {} phoneNo {}", sessionID, maskMsisdn(phoneNo)); logger.info("Record missed for sessionID {} phoneNo {}", sessionID, maskMsisdn(phoneNo));Also applies to: 112-112
src/main/java/com/iemr/common/utils/JwtUtil.java (1)
59-78: Minor: confirm expiration units and consider explicit algorithm
- Javadoc says βmillisecondsβ; verify properties are indeed in ms to avoid overly long/short tokens.
- Optional: explicitly specify the HMAC algorithm when signing to avoid version-dependent inference.
Would you like me to scan property files and config docs to confirm units and JJWT version?
src/main/resources/application.properties (3)
225-227: Gate Swagger with env/profile to avoid exposure in prod.Avoid hard-on swagger; prefer env toggle.
Apply:
-springdoc.api-docs.enabled=true -springdoc.swagger-ui.enabled=true +springdoc.api-docs.enabled=${SWAGGER_ENABLED:false} +springdoc.swagger-ui.enabled=${SWAGGER_ENABLED:false}Also applies to: 353-355
366-366: Trim leading space and validate wildcard in CORS.Leading space after '=' can sneak into value parsing; also confirm your CORS code actually supports patterns like localhost:*.
Apply:
-cors.allowed-origins= http://localhost:*,https://amritdemo.piramalswasthya.org +cors.allowed-origins=http://localhost:*,https://amritdemo.piramalswasthya.orgPlease confirm the binder for cors.allowed-origins trims/normalizes and supports globs; otherwise enumerate explicit dev ports.
176-181: Move βlocal envβ overrides to a profile-specific file.Prefer application-local.properties (activated via spring.profiles.active=local) to avoid mixing environment overrides into the base properties and accidental promotion.
I can generate a small PR to split these into application-local.properties and wire profile activation.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (5)
378-378: Remove stdout logging.Use the logger or drop the line; avoid printing PII.
- System.out.println(mUser); + // avoid printing PII in logs
299-306: Rotate refresh tokens by revoking the old JTI.Delete the previous entry to prevent reuse.
String newRefreshToken = jwtUtil.generateRefreshToken(user.getUserName(), userId); String newJti = jwtUtil.getJtiFromToken(newRefreshToken); + // revoke old refresh token + redisTemplate.delete("refresh:" + jti); redisTemplate.opsForValue().set( "refresh:" + newJti, userId, jwtUtil.getRefreshTokenExpiration(), TimeUnit.MILLISECONDS );
1174-1183: Use the shared constant for the JWT cookie name.Keeps header/cookie handling consistent.
- if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { + if (Constants.JWT_TOKEN.equalsIgnoreCase(cookie.getName())) {
765-765: Fix typo in operation summary.Public-facing text.
- @Operation(summary = "Get security quetions") + @Operation(summary = "Get security questions")
827-835: Correct log messages for getUsersByProviderID.Improves observability clarity.
- logger.info("getRolesByProviderID request "); + logger.info("getUsersByProviderID request "); @@ - logger.error("getRolesByProviderID failed with error " + e.getMessage(), e); + logger.error("getUsersByProviderID failed with error " + e.getMessage(), e); @@ - logger.info("getRolesByProviderID response " + response.toString()); + logger.info("getUsersByProviderID response " + response.toString());
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π‘ Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
π Files selected for processing (9)
src/main/environment/common_ci.properties(2 hunks)src/main/environment/common_docker.properties(2 hunks)src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java(2 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(14 hunks)src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandlerImpl.java(1 hunks)src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java(1 hunks)src/main/java/com/iemr/common/service/welcomeSms/WelcomeBenificarySmsServiceImpl.java(1 hunks)src/main/java/com/iemr/common/utils/JwtUtil.java(1 hunks)src/main/resources/application.properties(1 hunks)
π§ Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/iemr/common/service/welcomeSms/WelcomeBenificarySmsServiceImpl.java
- src/main/environment/common_ci.properties
- src/main/environment/common_docker.properties
- src/main/java/com/iemr/common/service/beneficiaryOTPHandler/BeneficiaryOTPHandlerImpl.java
- src/main/java/com/iemr/common/service/firebaseNotification/FirebaseNotificationService.java
π§° Additional context used
π§ Learnings (5)
π Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/utils/JwtUtil.java
π Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-02-03T12:41:59.244Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_uat.properties:0-0
Timestamp: 2025-02-03T12:41:59.244Z
Learning: In the Common-API project, JWT secret configuration should use environment variables (e.g., `jwt.secret=env.JWT_SECRET_KEY@`) across all environments for better security.
Applied to files:
src/main/java/com/iemr/common/utils/JwtUtil.javasrc/main/resources/application.properties
π Learning: 2025-02-03T12:41:45.354Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_test.properties:0-0
Timestamp: 2025-02-03T12:41:45.354Z
Learning: In Common-API, JWT secrets should be configured using placeholders (e.g., `<Enter_Your_Secret_Key>`) in properties files, similar to other sensitive credentials, to prevent accidental commit of actual secrets.
Applied to files:
src/main/java/com/iemr/common/utils/JwtUtil.javasrc/main/resources/application.properties
π Learning: 2025-01-30T09:11:34.734Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_example.properties:0-0
Timestamp: 2025-01-30T09:11:34.734Z
Learning: In example/template properties files, sensitive values like JWT secrets should use placeholders (e.g. `<Enter_Your_Secret_Key>`) instead of actual values to avoid exposing secrets in version control.
Applied to files:
src/main/resources/application.properties
πͺ GitHub Check: SonarCloud
src/main/resources/application.properties
[failure] 220-220: Credentials should not be hard-coded
Revoke and change this password, as it is compromised.See more on SonarQube Cloud
[failure] 371-371: Credentials should not be hard-coded
Revoke and change this password, as it is compromised.See more on SonarQube Cloud
[failure] 371-371: Database passwords should not be disclosed
Make sure this database password gets changed and removed from the code.See more on SonarQube Cloud
[failure] 375-375: Database passwords should not be disclosed
Make sure this database password gets changed and removed from the code.See more on SonarQube Cloud
π Additional comments (10)
src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java (1)
184-184: EOF newline addition is finesrc/main/java/com/iemr/common/utils/JwtUtil.java (7)
37-47: Access token generation looks goodParameters and delegation are clear; no issues spotted.
48-57: Refresh token generation looks goodConsistent with access token; no issues spotted.
118-129: Utility accessor is cleanGeneric extractor is fine; no issues.
130-137: JTI accessor looks goodMatches JTI set during buildToken.
139-147: Username accessor looks goodStraightforward subject extraction.
157-164: Refresh expiration accessor looks goodExposes configured TTL; no issues.
104-117: Enforce token validation before extracting claims
getAllClaimsFromToken and its helpers skip denylist/type checks. Ensure callers in FirebaseNotificationService (line 113) and IEMRAdminController (lines 261, 584) invoke jwtUtil.validateToken or the appropriate typed variant (validateAccessToken/validateRefreshToken) before any get*FromToken calls.src/main/resources/application.properties (1)
383-386: Specify or removesms-consent-source-address
Bindingsms-consent-source-addresswithout an β=valueβ loads it as an empty string, causing the OTP-consent SMS to have a blank source and likely fail. Either set an explicit default:-sms-consent-source-address +sms-consent-source-address=${SMS_CONSENT_SOURCE_ADDR:}or remove the key entirely if it isnβt used.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
981-989: Constants.JWT_TOKEN is correctly defined as βJwttokenβ and used consistently across the codebase.
| // Extract token from cookies or headers | ||
| String token = getJwtTokenFromCookies(httpRequest); | ||
| if (token == null) { | ||
| response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
| outputResponse.setError(new RuntimeException("No JWT token found in request")); | ||
| return outputResponse.toString(); | ||
| } | ||
|
|
||
| // Validate the token: Check if it is expired or in the deny list | ||
| Claims claims = jwtUtil.validateToken(token); | ||
| if (claims.isEmpty() || claims.getExpiration() == null || claims.getId() == null) { // If token is either expired or in the deny list, return 401 Unauthorized | ||
| response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); | ||
| outputResponse.setError(new RuntimeException("Token is expired or has been logged out")); | ||
| return outputResponse.toString(); | ||
| } | ||
|
|
||
| // Extract the jti (JWT ID) and expiration time from the validated claims | ||
| String jti = claims.getId(); // jti is in the 'id' field of claims | ||
| long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims | ||
| long ttlMillis = expirationTime - System.currentTimeMillis(); | ||
| tokenDenylist.addTokenToDenylist(jti, ttlMillis); | ||
|
|
There was a problem hiding this comment.
Harden forceLogout: support headers, null-safe claims, and guard TTL.
Prevents NPE and accepts token from common sources.
- // Extract token from cookies or headers
- String token = getJwtTokenFromCookies(httpRequest);
- if (token == null) {
- response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
- outputResponse.setError(new RuntimeException("No JWT token found in request"));
- return outputResponse.toString();
- }
+ // Extract token from cookies or headers
+ String token = getJwtTokenFromCookies(httpRequest);
+ if (token == null) {
+ token = httpRequest.getHeader(Constants.JWT_TOKEN);
+ }
+ if (token == null) {
+ String authz = httpRequest.getHeader("Authorization");
+ if (authz != null && authz.startsWith("Bearer ")) {
+ token = authz.substring(7);
+ }
+ }
+ if (token == null || token.isBlank()) {
+ response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+ outputResponse.setError(new RuntimeException("No JWT token found in request"));
+ return outputResponse.toString();
+ }
// Validate the token: Check if it is expired or in the deny list
Claims claims = jwtUtil.validateToken(token);
- if (claims.isEmpty() || claims.getExpiration() == null || claims.getId() == null) { // If token is either expired or in the deny list, return 401 Unauthorized
+ if (claims == null || claims.getExpiration() == null || claims.getId() == null) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
outputResponse.setError(new RuntimeException("Token is expired or has been logged out"));
return outputResponse.toString();
}
// Extract the jti (JWT ID) and expiration time from the validated claims
String jti = claims.getId(); // jti is in the 'id' field of claims
long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims
long ttlMillis = expirationTime - System.currentTimeMillis();
- tokenDenylist.addTokenToDenylist(jti, ttlMillis);
+ if (ttlMillis > 0) {
+ tokenDenylist.addTokenToDenylist(jti, ttlMillis);
+ }π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Extract token from cookies or headers | |
| String token = getJwtTokenFromCookies(httpRequest); | |
| if (token == null) { | |
| response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | |
| outputResponse.setError(new RuntimeException("No JWT token found in request")); | |
| return outputResponse.toString(); | |
| } | |
| // Validate the token: Check if it is expired or in the deny list | |
| Claims claims = jwtUtil.validateToken(token); | |
| if (claims.isEmpty() || claims.getExpiration() == null || claims.getId() == null) { // If token is either expired or in the deny list, return 401 Unauthorized | |
| response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); | |
| outputResponse.setError(new RuntimeException("Token is expired or has been logged out")); | |
| return outputResponse.toString(); | |
| } | |
| // Extract the jti (JWT ID) and expiration time from the validated claims | |
| String jti = claims.getId(); // jti is in the 'id' field of claims | |
| long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims | |
| long ttlMillis = expirationTime - System.currentTimeMillis(); | |
| tokenDenylist.addTokenToDenylist(jti, ttlMillis); | |
| // Extract token from cookies or headers | |
| String token = getJwtTokenFromCookies(httpRequest); | |
| if (token == null) { | |
| token = httpRequest.getHeader(Constants.JWT_TOKEN); | |
| } | |
| if (token == null) { | |
| String authz = httpRequest.getHeader("Authorization"); | |
| if (authz != null && authz.startsWith("Bearer ")) { | |
| token = authz.substring(7); | |
| } | |
| } | |
| if (token == null || token.isBlank()) { | |
| response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | |
| outputResponse.setError(new RuntimeException("No JWT token found in request")); | |
| return outputResponse.toString(); | |
| } | |
| // Validate the token: Check if it is expired or in the deny list | |
| Claims claims = jwtUtil.validateToken(token); | |
| if (claims == null || claims.getExpiration() == null || claims.getId() == null) { | |
| response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); | |
| outputResponse.setError(new RuntimeException("Token is expired or has been logged out")); | |
| return outputResponse.toString(); | |
| } | |
| // Extract the jti (JWT ID) and expiration time from the validated claims | |
| String jti = claims.getId(); // jti is in the 'id' field of claims | |
| long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims | |
| long ttlMillis = expirationTime - System.currentTimeMillis(); | |
| if (ttlMillis > 0) { | |
| tokenDenylist.addTokenToDenylist(jti, ttlMillis); | |
| } |
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
around lines 950 to 971, the forceLogout flow should accept tokens from
Authorization headers as well as cookies, avoid NPEs when claims or fields are
null, and guard TTL before adding to the denylist: extract token first by
checking Authorization header (handle "Bearer " prefix) then cookies; call
jwtUtil.validateToken and null-check the returned Claims object and its
getExpiration() and getId() before using them; if any of those are missing
return 401 with the existing error handling; compute ttlMillis = expirationTime
- System.currentTimeMillis() and only call tokenDenylist.addTokenToDenylist(jti,
ttlMillis) if ttlMillis > 0 (or clamp to a minimal positive value), otherwise
skip adding or handle as immediate expiry.
| @Value("${jwt.secret}") | ||
| private String SECRET_KEY; | ||
|
|
||
| @Value("${jwt.access.expiration}") | ||
| private long ACCESS_EXPIRATION_TIME; | ||
|
|
||
| @Value("${jwt.refresh.expiration}") | ||
| private long REFRESH_EXPIRATION_TIME; | ||
|
|
There was a problem hiding this comment.
π‘ Verification agent
𧩠Analysis chain
Config hygiene: confirm env-backed jwt.secret across environments
Ensure all env/property files use placeholders/env-var indirection (e.g., jwt.secret=env.JWT_SECRET_KEY@) per team practice.
Run:
π Script executed:
#!/bin/bash
# Verify jwt.secret is configured via env-backed placeholder in all env files
rg -nP '^\s*jwt\.secret\s*=' src/main -C2Length of output: 1099
Replace hardcoded JWT secret in common_example.properties
common_example.properties (line 177) still contains a literal secret (my-32-character-ultra-secure-and-ultra-long-secret); update it to a placeholder (e.g. <Enter_Your_Secret_Key> or use @env.JWT_SECRET_KEY@).
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/utils/JwtUtil.java around lines 18 to 26, the
project example properties file still contains a hardcoded JWT secret; open
common_example.properties (line ~177) and replace the literal
"my-32-character-ultra-secure-and-ultra-long-secret" with a neutral placeholder
such as "<Enter_Your_Secret_Key>" or a tokenized variable like
"@env.JWT_SECRET_KEY@", and update any documentation or env var mapping so
consumers know to set the real secret in their environment or
application.properties instead of committing a real secret.
| @Autowired | ||
| private TokenDenylist tokenDenylist; | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Make TokenDenylist injection optional to avoid startup failures in profiles without the bean
If the bean isnβt present for some environments, the app will fail to start. Make it optional and guard the call.
Apply this diff:
- @Autowired
- private TokenDenylist tokenDenylist;
+ @Autowired(required = false)
+ private TokenDenylist tokenDenylist;π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Autowired | |
| private TokenDenylist tokenDenylist; | |
| // Make TokenDenylist optional to avoid startup failures when the bean is absent | |
| - @Autowired | |
| @Autowired(required = false) | |
| private TokenDenylist tokenDenylist; |
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/utils/JwtUtil.java around lines 27 to 29, the
TokenDenylist field is currently required and will cause startup failures when
that bean is absent; make the injection optional (e.g. use @Autowired(required =
false) or inject Optional<TokenDenylist> / @Nullable TokenDenylist) and update
any usages to null-check or use Optional.ifPresent before calling methods on it
so code paths that rely on the denylist are guarded when the bean is not
provided.
| identity-api-url = http://localhost:8094/ | ||
| #Verify whether 1097 and identity are same? | ||
| identity-1097-api-url = http://localhost:8095/ | ||
| ##Generate Benificiary Config | ||
| genben-api=http://localhost:8092/ | ||
|
|
||
| #### SMS Configuration | ||
| send-sms=false | ||
| sendSMSUrl = http://localhost:8080/sms/sendSMS | ||
| source-address=AIDSHL |
There was a problem hiding this comment.
π οΈ Refactor suggestion
SMS/Email credentials should be env-driven, not literals/placeholders.
Keep this file deploy-safe; bind sensitive values from environment.
Apply:
-sendSMSUrl = http://localhost:8080/sms/sendSMS
-source-address=AIDSHL
-sms-username=<Enter SMS username>
-sms-password=<Enter SMS password>
-send-message-url=<Enter SMS sending URL here>
+sendSMSUrl=${SMS_SEND_ENDPOINT:http://localhost:8080/sms/sendSMS}
+source-address=${SMS_SOURCE_ADDR:AIDSHL}
+sms-username=${SMS_USERNAME:}
+sms-password=${SMS_PASSWORD:}
+send-message-url=${SMS_SEND_URL:}
-spring.mail.username=<Enter mail username>
-spring.mail.password=<Enter mail password>
+spring.mail.username=${MAIL_USERNAME:}
+spring.mail.password=${MAIL_PASSWORD:}Also applies to: 215-223
π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 197 to 206 (and also
apply same changes to lines 215-223), sensitive SMS/email credentials and URLs
must not be hardcoded; replace literal values with environment-driven property
placeholders (e.g. use Spring property placeholders that read from environment
variables with safe defaults like ${SMS_SEND:false} or
${SEND_SMS_URL:http://localhost:8080/sms/sendSMS}) and remove any plaintext
secrets; ensure keys remain the same but values reference environment variables
(document the required ENV names) so credentials are injected at runtime rather
than stored in the file.
| spring.redis.host=localhost | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Parameterize Redis via env.
Prevents accidental commits of host/secret and eases deploys.
Apply:
-spring.redis.host=localhost
+spring.redis.host=${REDIS_HOST:localhost}Additionally set password (line 102) from env:
spring.redis.password=${REDIS_PASSWORD:}π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 263-264 the Redis host
is hardcoded to "localhost", which should be parameterized via environment
variables; update the property to read from an env var with a sensible default
(e.g. ${REDIS_HOST:localhost}) and likewise change the Redis password property
(around line 102) to pull from env (e.g. ${REDIS_PASSWORD:}) so secrets and host
values are not committed.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (4)
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java (1)
17-18: Add the missingstart-grievancedatasync-schedulerconfig key or guard the scheduler bean
The flag property isnβt defined (onlycron-scheduler-grievancedatasyncexists), which will cause injection failures at startup; either definestart-grievancedatasync-schedulerin all application*.properties/yml across environments or annotate the bean with@ConditionalOnProperty("start-grievancedatasync-scheduler").src/main/resources/application.properties (3)
101-103: Parameterize Redis host/password.Avoid committing credentials and ease environment overrides.
-spring.redis.password= +spring.redis.password=${REDIS_PASSWORD:} -spring.redis.host=localhost +spring.redis.host=${REDIS_HOST:localhost}Also applies to: 263-264
1-386: Remove hard-coded secrets from default configs and enforce env-driven valuesHard-coded credentials detected in
application.propertiesandcommon_example.properties. Switch all secrets (passwords, API keys, salts, tokens, JWT secret, encrypted values) to environment variables:
- application.properties:
km-password,km-guest-password,sms-password,spring.mail.password,eSanjeevani.password- common_example.properties:
spring.datasource.password,secondary.datasource.password,encDbUserName,km-password,km-guest-password,jwt.secretξcommon_example.properties lines 6, 13, 21, 26, 44, 56, 163β164ξ
common_ci.properties and common_docker.properties correctly use@envβ¦@/${β¦}forjwt.secretand other secrets; mirror that pattern here.
1-20: Replace deprecated Spring Boot properties
- Remove
spring.http.multipart.max-request-sizeandspring.http.multipart.max-file-size(removed in Boot 2.0+) and replace them withspring.servlet.multipart.max-request-sizeandspring.servlet.multipart.max-file-size.- Remove
spring.jpa.hibernate.naming_strategy(ignored in Boot 3.x); rely solely onspring.jpa.hibernate.naming.implicit-strategyandspring.jpa.hibernate.naming.physical-strategy.
β»οΈ Duplicate comments (3)
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java (1)
17-18: Default to false and avoid startup failure when property is missing.Without a default, missing property will fail context startup; safe default should be disabled. This echoes a prior review.
Apply:
- @Value("${start-grievancedatasync-scheduler}") + @Value("${start-grievancedatasync-scheduler:false}") private boolean grievanceFlag;src/main/resources/application.properties (1)
369-377: DB credentials env-driven; update deprecated MySQL driver.Use env placeholders and modern driver class.
-spring.datasource.url=jdbc:mysql://localhost:3306/db_iemr -spring.datasource.username=<Enter userName> -spring.datasource.password=<Enter password> +spring.datasource.url=${DB_URL:jdbc:mysql://localhost:3306/db_iemr} +spring.datasource.username=${DB_USERNAME:} +spring.datasource.password=${DB_PASSWORD:} -secondary.datasource.username=<Enter username> -secondary.datasource.password=<Enter password> -secondary.datasource.url=jdbc:mysql://localhost:3306/db_reporting -secondary.datasource.driver-class-name=com.mysql.jdbc.Driver +secondary.datasource.username=${DB2_USERNAME:} +secondary.datasource.password=${DB2_PASSWORD:} +secondary.datasource.url=${DB2_URL:jdbc:mysql://localhost:3306/db_reporting} +secondary.datasource.driver-class-name=com.mysql.cj.jdbc.Driversrc/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
944-978: Force logout implementation needs hardening against security issues.The current implementation has several security issues noted in past reviews:
- Only checks cookies for JWT tokens, ignoring Authorization headers
- Doesn't null-check the Claims object from
validateToken- Uses
claims.isEmpty()when Claims objects don't have anisEmpty()method- Doesn't guard against negative TTL values
The implementation still needs the fixes suggested in previous reviews to handle multiple token sources, null-safe claims validation, and proper TTL handling.
π§Ή Nitpick comments (8)
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java (3)
12-16: Prefer property-gated bean creation over runtime flag checks.Using ConditionalOnProperty prevents the bean from loading when disabled and also avoids cron placeholder resolution when not needed.
Apply:
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -@Component +@Component +@ConditionalOnProperty(value = "start-grievancedatasync-scheduler", havingValue = "true", matchIfMissing = false) public class ScheduleForGrievanceDataSync {Optionally remove the grievanceFlag field and the if-block if you adopt this.
20-21: Make logger static final and use class literal.Minor consistency/perf nit.
- private final Logger logger = LoggerFactory.getLogger(this.getClass().getName()); + private static final Logger logger = LoggerFactory.getLogger(ScheduleForGrievanceDataSync.class);
29-35: Harden scheduled run with try/catch and consistent logs.Prevents partial logs and surfaces failures clearly.
@Scheduled(cron = "${cron-scheduler-grievancedatasync}") public void execute() { - if (grievanceFlag) { - logger.info("Started job for grievance data sync "); - grievanceDataSync.dataSyncToGrievance(); - logger.info("Completed job for grievance data sync "); - } + if (grievanceFlag) { + logger.info("Started job for grievance data sync"); + try { + grievanceDataSync.dataSyncToGrievance(); + logger.info("Completed job for grievance data sync"); + } catch (Exception e) { + logger.error("Grievance data sync failed", e); + } + } else { + logger.debug("Grievance data sync scheduler disabled by config"); + } }src/main/resources/application.properties (3)
191-200: CTI IPs/URLs: avoid committing real internal addresses.Parameterize to prevent leakage and ease environment swaps.
-cti-server-ip=10.208.122.99 -cti-logger_base_url=http://10.208.122.99/logger +cti-server-ip=${CTI_SERVER_IP:127.0.0.1} +cti-logger_base_url=${CTI_LOGGER_BASE_URL:http://127.0.0.1/logger} -identity-api-url = http://localhost:8094/ -identity-1097-api-url = http://localhost:8095/ +identity-api-url = ${IDENTITY_BASE_URL:http://localhost:8094/} +identity-1097-api-url = ${IDENTITY_1097_BASE_URL:http://localhost:8095/}
366-366: Tiny nit: drop leading space after '=' in CORS origins.Prevents accidental whitespace parsing issues in custom loaders.
-cors.allowed-origins= http://localhost:*,https://amritdemo.piramalswasthya.org +cors.allowed-origins=http://localhost:*,https://amritdemo.piramalswasthya.org
379-379: Make video-call-url configurable.-video-call-url = +video-call-url = ${VIDEO_CALL_URL:}src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
955-955: Fix indentation inconsistency.The return statement indentation doesn't match the surrounding code style.
- return outputResponse.toString(); + return outputResponse.toString();
992-992: Remove unnecessary blank line.Extra blank line should be removed for consistency.
- @Operation(summary = "User force log out")
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π‘ Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
π Files selected for processing (10)
src/main/environment/common_ci.properties(3 hunks)src/main/environment/common_docker.properties(2 hunks)src/main/java/com/iemr/common/CommonApplication.java(0 hunks)src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java(1 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(13 hunks)src/main/java/com/iemr/common/model/beneficiary/BeneficiaryModel.java(1 hunks)src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java(5 hunks)src/main/java/com/iemr/common/utils/JwtUtil.java(2 hunks)src/main/resources/application.properties(1 hunks)
π€ Files with no reviewable changes (1)
- src/main/java/com/iemr/common/CommonApplication.java
β Files skipped from review due to trivial changes (2)
- src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java
- src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java
π§ Files skipped from review as they are similar to previous changes (4)
- src/main/environment/common_ci.properties
- src/main/java/com/iemr/common/model/beneficiary/BeneficiaryModel.java
- src/main/environment/common_docker.properties
- src/main/java/com/iemr/common/utils/JwtUtil.java
π§° Additional context used
π§ Learnings (5)
π Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-02-03T12:41:59.244Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_uat.properties:0-0
Timestamp: 2025-02-03T12:41:59.244Z
Learning: In the Common-API project, JWT secret configuration should use environment variables (e.g., `jwt.secret=env.JWT_SECRET_KEY@`) across all environments for better security.
Applied to files:
src/main/resources/application.properties
π Learning: 2025-02-03T12:41:45.354Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_test.properties:0-0
Timestamp: 2025-02-03T12:41:45.354Z
Learning: In Common-API, JWT secrets should be configured using placeholders (e.g., `<Enter_Your_Secret_Key>`) in properties files, similar to other sensitive credentials, to prevent accidental commit of actual secrets.
Applied to files:
src/main/resources/application.properties
π Learning: 2025-01-30T09:11:34.734Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/environment/common_example.properties:0-0
Timestamp: 2025-01-30T09:11:34.734Z
Learning: In example/template properties files, sensitive values like JWT secrets should use placeholders (e.g. `<Enter_Your_Secret_Key>`) instead of actual values to avoid exposing secrets in version control.
Applied to files:
src/main/resources/application.properties
πͺ GitHub Check: SonarCloud
src/main/resources/application.properties
[failure] 220-220: Credentials should not be hard-coded
Revoke and change this password, as it is compromised.See more on SonarQube Cloud
[failure] 371-371: Credentials should not be hard-coded
Revoke and change this password, as it is compromised.See more on SonarQube Cloud
[failure] 371-371: Database passwords should not be disclosed
Make sure this database password gets changed and removed from the code.See more on SonarQube Cloud
[failure] 375-375: Database passwords should not be disclosed
Make sure this database password gets changed and removed from the code.See more on SonarQube Cloud
π Additional comments (8)
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java (1)
40-41: No-op formatting change.Trailing newline only. No functional impact.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (7)
83-84: LGTM! Configuration restored from hardcoded value.The
enableCaptchafield is now properly injected from external configuration using@Value, which addresses the previous hardcoded issue and allows for environment-specific CAPTCHA control.
173-175: Authentication logic tightened with withCredentials requirement.The condition now requires
withCredentialsto be true for concurrent session checks, which improves authentication flow control by ensuring explicit credential validation.
190-190: Code formatting cleaned up.Trailing whitespace has been removed from these lines, improving code consistency.
Also applies to: 281-281, 287-287, 297-297, 469-469
980-990: Helper method implementation is clean.The
getJwtTokenFromCookieshelper method properly iterates through cookies and uses the correct constant for JWT token name matching.
994-996: Function signature simplified correctly.The
userForceLogoutmethod signature has been simplified to only accept theForceLogoutRequestModel, removing the HttpServletRequest and HttpServletResponse parameters as indicated in the AI summary.
1169-1187: New JWT token retrieval endpoint implementation.The new
get-jwt-tokenendpoint provides a clean way to retrieve JWT tokens from cookies. The implementation correctly:
- Uses proper HTTP status codes (200 for found, 404 for not found)
- Searches cookies with case-insensitive matching
- Returns appropriate response messages
1189-1213: Helper method for authentication response preparation.The new
prepareAuthenticationResponsehelper method cleanly separates the response preparation logic and properly handles null user scenarios.
| // Try JWT token from header first | ||
| String jwtToken = request.getHeader("Jwttoken"); | ||
|
|
There was a problem hiding this comment.
JWT token extraction logic needs validation hardening.
The token extraction logic attempts to get JWT from headers and cookies but doesn't validate the token before using it. This creates a security risk as getUserIdFromToken is called on an unvalidated token.
Apply this fix to validate the token before extracting user ID:
// If not in header, try cookie
if (jwtToken == null) {
Cookie[] cookies = request.getCookies();
if (cookies != null) {
for (Cookie cookie : cookies) {
if ("Jwttoken".equalsIgnoreCase(cookie.getName())) {
jwtToken = cookie.getValue();
break;
}
}
}
}
if (jwtToken == null) {
logger.warn("Authentication failed: no token found in header or cookies.");
throw new IEMRException("Authentication failed. Please log in again.");
}
- // Extract user ID from the JWT token
- String userId = jwtUtil.getUserIdFromToken(jwtToken);
+ // Validate token first, then extract user ID
+ Claims claims = jwtUtil.validateToken(jwtToken);
+ if (claims == null) {
+ logger.warn("Authentication failed: invalid token provided.");
+ throw new IEMRException("Authentication failed. Please log in again.");
+ }
+ String userId = claims.get("userId", String.class);Also applies to: 577-577, 582-582, 585-585
| km-base-protocol=http | ||
| km-username=okmAdmin | ||
| km-password=admin | ||
| km-base-url=http://localhost:8084/OpenKM | ||
| km-base-path=localhost:8084/OpenKM | ||
| km-root-path=/okm:personal/users/ | ||
| km-guest-user=guest | ||
| km-guest-password=guest | ||
|
|
There was a problem hiding this comment.
Hard-coded KM/OpenKM credentials; externalize via env now.
Replace literals with env-driven placeholders to remove secrets from VCS and align with team standards.
-km-base-protocol=http
-km-username=okmAdmin
-km-password=admin
-km-base-url=http://localhost:8084/OpenKM
-km-base-path=localhost:8084/OpenKM
-km-root-path=/okm:personal/users/
-km-guest-user=guest
-km-guest-password=guest
+km-base-protocol=${KM_BASE_PROTOCOL:http}
+km-username=${KM_USERNAME:}
+km-password=${KM_PASSWORD:}
+km-base-url=${KM_BASE_URL:http://localhost:8084/OpenKM}
+km-base-path=${KM_BASE_PATH:localhost:8084/OpenKM}
+km-root-path=${KM_ROOT_PATH:/okm:personal/users/}
+km-guest-user=${KM_GUEST_USER:}
+km-guest-password=${KM_GUEST_PASSWORD:}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| km-base-protocol=http | |
| km-username=okmAdmin | |
| km-password=admin | |
| km-base-url=http://localhost:8084/OpenKM | |
| km-base-path=localhost:8084/OpenKM | |
| km-root-path=/okm:personal/users/ | |
| km-guest-user=guest | |
| km-guest-password=guest | |
| # src/main/resources/application.properties (lines 182β190) | |
| -km-base-protocol=http | |
| -km-username=okmAdmin | |
| -km-password=admin | |
| -km-base-url=http://localhost:8084/OpenKM | |
| -km-base-path=localhost:8084/OpenKM | |
| -km-root-path=/okm:personal/users/ | |
| -km-guest-user=guest | |
| km-base-protocol=${KM_BASE_PROTOCOL:http} | |
| km-username=${KM_USERNAME:} | |
| km-password=${KM_PASSWORD:} | |
| km-base-url=${KM_BASE_URL:http://localhost:8084/OpenKM} | |
| km-base-path=${KM_BASE_PATH:localhost:8084/OpenKM} | |
| km-root-path=${KM_ROOT_PATH:/okm:personal/users/} | |
| km-guest-user=${KM_GUEST_USER:} | |
| km-guest-password=${KM_GUEST_PASSWORD:} |
π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 182 to 190 the
KM/OpenKM credentials and URLs are hard-coded; replace those literal values with
environment-driven placeholders (Spring property placeholders or profiles) so
secrets are not stored in VCS. Update each key (km-username, km-password,
km-base-url, km-base-path, km-root-path, km-guest-user, km-guest-password) to
reference env variables (e.g., ${KM_USERNAME:default} style) and ensure any
code/config that reads these properties still works; add guidance to document
required env variable names and defaults in README or a sample
application.properties.dist.
| #### SMS Configuration | ||
| send-sms=false | ||
| sendSMSUrl = http://localhost:8080/sms/sendSMS | ||
| source-address=AIDSHL | ||
| sms-username=<Enter SMS username> | ||
| sms-password=<Enter SMS password> | ||
| send-message-url=<Enter SMS sending URL here> | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
SMS/Email credentials must be env-driven.
Prevent secret exposure; keep deploy-safe defaults.
-send-sms=false
-sendSMSUrl = http://localhost:8080/sms/sendSMS
-source-address=AIDSHL
-sms-username=<Enter SMS username>
-sms-password=<Enter SMS password>
-send-message-url=<Enter SMS sending URL here>
+send-sms=${SEND_SMS:false}
+sendSMSUrl=${SMS_SEND_ENDPOINT:http://localhost:8080/sms/sendSMS}
+source-address=${SMS_SOURCE_ADDR:AIDSHL}
+sms-username=${SMS_USERNAME:}
+sms-password=${SMS_PASSWORD:}
+send-message-url=${SMS_SEND_URL:}
-send-email=false
-spring.mail.host=smtp.gmail.com
-spring.mail.port=587
-spring.mail.username=<Enter mail username>
-spring.mail.password=<Enter mail password>
+send-email=${SEND_EMAIL:false}
+spring.mail.host=${MAIL_HOST:smtp.gmail.com}
+spring.mail.port=${MAIL_PORT:587}
+spring.mail.username=${MAIL_USERNAME:}
+spring.mail.password=${MAIL_PASSWORD:}Also applies to: 215-223
π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 203-210 (and also
apply same change to 215-223), the SMS credentials and URLs are hard-coded,
exposing secrets; change these entries to use environment-driven placeholders
(Spring property placeholders) and safe defaults β e.g. replace sms-username,
sms-password, sendSMSUrl, send-message-url and source-address with expressions
like sms.username=${SMS_USERNAME:}, sms.password=${SMS_PASSWORD:},
sms.send-url=${SMS_SEND_URL:http://localhost:8080/sms/sendSMS},
sms.source-address=${SMS_SOURCE_ADDRESS:AIDSHL} so the app reads secrets from
environment variables with an empty or non-sensitive default, remove literal
secret values, and document in README or deployment manifests which env vars
must be set.
| everwellUserName = <Enter everwell username> | ||
| everwellPassword = <Enter everwell password> | ||
| amritUserName = <Enter AMRIT username> | ||
| amritPassword = <Enter AMRIT password> | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Everwell/AMRIT credentials as env vars.
-everwellUserName = <Enter everwell username>
-everwellPassword = <Enter everwell password>
-amritUserName = <Enter AMRIT username>
-amritPassword = <Enter AMRIT password>
+everwellUserName = ${EVERWELL_USERNAME:}
+everwellPassword = ${EVERWELL_PASSWORD:}
+amritUserName = ${AMRIT_USERNAME:}
+amritPassword = ${AMRIT_PASSWORD:}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| everwellUserName = <Enter everwell username> | |
| everwellPassword = <Enter everwell password> | |
| amritUserName = <Enter AMRIT username> | |
| amritPassword = <Enter AMRIT password> | |
| # In src/main/resources/application.properties, replace lines 296β300 with: | |
| everwellUserName = ${EVERWELL_USERNAME:} | |
| everwellPassword = ${EVERWELL_PASSWORD:} | |
| amritUserName = ${AMRIT_USERNAME:} | |
| amritPassword = ${AMRIT_PASSWORD:} |
π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 296 to 300, the
Everwell/AMRIT plaintext credentials are committed; replace these hardcoded
values with environment-backed properties (e.g. use Spring placeholders like
everwell.username=${EVERWELL_USERNAME} and
everwell.password=${EVERWELL_PASSWORD}, and similarly
amrit.username=${AMRIT_USERNAME} and amrit.password=${AMRIT_PASSWORD}), remove
the literal "<Enter ...>" values, and update any docs or deployment manifests to
set those environment variables; ensure no secrets remain in the repo or in this
file and add guidance to use a secrets manager or CI/CD secret store where
appropriate.
| ## LungAssessment credentials | ||
| lungAssessmentEmail = <Enter SWAASA email id> | ||
| lungAssessmentPassword = <Enter SWAASA password> | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
SWAASA credentials should not live in repo.
-lungAssessmentEmail = <Enter SWAASA email id>
-lungAssessmentPassword = <Enter SWAASA password>
+lungAssessmentEmail = ${SWAASA_EMAIL:}
+lungAssessmentPassword = ${SWAASA_PASSWORD:}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## LungAssessment credentials | |
| lungAssessmentEmail = <Enter SWAASA email id> | |
| lungAssessmentPassword = <Enter SWAASA password> | |
| ## LungAssessment credentials | |
| -lungAssessmentEmail = <Enter SWAASA email id> | |
| lungAssessmentEmail = ${SWAASA_EMAIL:} | |
| lungAssessmentPassword = ${SWAASA_PASSWORD:} |
π€ Prompt for AI Agents
In src/main/resources/application.properties lines 312-315, remove the
hard-coded SWAASA email/password entries so credentials do not live in the repo;
instead replace them with references to environment variables or an external
secret store (e.g., use Spring Boot property placeholders that read from
env/secret manager), create an application.properties.example with non-sensitive
placeholders for onboarding, update README with how to set the required env vars
or integrate the secret manager, and ensure any files containing real
credentials are not committed and are excluded via .gitignore or
secret-management tooling.
| eSanjeevani.url: https://preprod.esanjeevaniopd.xyz/uat/aus/api/ThirdPartyAuth/providerLogin | ||
| eSanjeevani.userName: <Enter e-sanjeevani username> | ||
| eSanjeevani.password: <Enter e-sanjeevani password> | ||
| eSanjeevani.salt: 123456 | ||
| eSanjeevani.source: 11001 | ||
| eSanjeevani.registerPatient: https://preprod.esanjeevaniopd.xyz/uat/ps/api/v1/Patient | ||
| eSanjeevani.routeUrl: https://uat.esanjeevani.in/user/signin | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
eSanjeevani secrets to env; keep configurable salt/source.
-eSanjeevani.userName: <Enter e-sanjeevani username>
-eSanjeevani.password: <Enter e-sanjeevani password>
-eSanjeevani.salt: 123456
-eSanjeevani.source: 11001
+eSanjeevani.userName: ${ESANJEEVANI_USERNAME:}
+eSanjeevani.password: ${ESANJEEVANI_PASSWORD:}
+eSanjeevani.salt: ${ESANJEEVANI_SALT:123456}
+eSanjeevani.source: ${ESANJEEVANI_SOURCE:11001}π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 324 to 331, the
eSanjeevani username and password are stored inline; change eSanjeevani.userName
and eSanjeevani.password to reference environment-backed placeholders (e.g.
${ESANJEEVANI_USERNAME:} and ${ESANJEEVANI_PASSWORD:}) so secrets come from env
vars and are not committed, leave eSanjeevani.salt and eSanjeevani.source as
configurable properties in this file (do not hardcode secrets), and add a note
to the README or deployment docs to set the corresponding environment variables
in each environment.
| grievanceUserName = <Enter Grievance username> | ||
| grievancePassword = <Enter Grievance password> | ||
|
|
||
| grievanceUserAuthenticate = <Enter Grievance UserAuthenticate> | ||
| grievanceDataSyncDuration = <Enter Grievience Data Sync Duration> |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Grievance service credentials β env variables.
-grievanceUserName = <Enter Grievance username>
-grievancePassword = <Enter Grievance password>
-grievanceUserAuthenticate = <Enter Grievance UserAuthenticate>
-grievanceDataSyncDuration = <Enter Grievience Data Sync Duration>
+grievanceUserName = ${GRIEVANCE_USERNAME:}
+grievancePassword = ${GRIEVANCE_PASSWORD:}
+grievanceUserAuthenticate = ${GRIEVANCE_AUTH_URL:}
+grievanceDataSyncDuration = ${GRIEVANCE_SYNC_MINUTES:}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| grievanceUserName = <Enter Grievance username> | |
| grievancePassword = <Enter Grievance password> | |
| grievanceUserAuthenticate = <Enter Grievance UserAuthenticate> | |
| grievanceDataSyncDuration = <Enter Grievience Data Sync Duration> | |
| # src/main/resources/application.properties (around lines 347β351) | |
| -grievanceUserName = <Enter Grievance username> | |
| -grievancePassword = <Enter Grievance password> | |
| -grievanceUserAuthenticate = <Enter Grievance UserAuthenticate> | |
| grievanceUserName = ${GRIEVANCE_USERNAME:} | |
| grievancePassword = ${GRIEVANCE_PASSWORD:} | |
| grievanceUserAuthenticate = ${GRIEVANCE_AUTH_URL:} | |
| grievanceDataSyncDuration = ${GRIEVANCE_SYNC_MINUTES:} |
| captcha.secret-key= <Enter Cloudflare Secret Key> | ||
| captcha.verify-url= https://challenges.cloudflare.com/turnstile/v0/siteverify | ||
| captcha.enable-captcha=false | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Captcha secret must be env-sourced.
-captcha.secret-key= <Enter Cloudflare Secret Key>
-captcha.verify-url= https://challenges.cloudflare.com/turnstile/v0/siteverify
-captcha.enable-captcha=false
+captcha.secret-key=${CAPTCHA_SECRET_KEY:}
+captcha.verify-url=${CAPTCHA_VERIFY_URL:https://challenges.cloudflare.com/turnstile/v0/siteverify}
+captcha.enable-captcha=${CAPTCHA_ENABLED:false}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| captcha.secret-key= <Enter Cloudflare Secret Key> | |
| captcha.verify-url= https://challenges.cloudflare.com/turnstile/v0/siteverify | |
| captcha.enable-captcha=false | |
| # In src/main/resources/application.properties around lines 362-365 | |
| -captcha.secret-key= <Enter Cloudflare Secret Key> | |
| -captcha.verify-url= https://challenges.cloudflare.com/turnstile/v0/siteverify | |
| captcha.secret-key=${CAPTCHA_SECRET_KEY:} | |
| captcha.verify-url=${CAPTCHA_VERIFY_URL:https://challenges.cloudflare.com/turnstile/v0/siteverify} | |
| captcha.enable-captcha=${CAPTCHA_ENABLED:false} |
π€ Prompt for AI Agents
In src/main/resources/application.properties around lines 362-365, the
captcha.secret-key is currently hardcoded/placeholder text; change it to load
the secret from an environment-backed property (e.g. reference a placeholder
that resolves from an env var), remove any literal secret from the repo, and
update docs/deployment configs to set that environment variable (ensure
local/dev fallback or require explicit setting and keep captcha.enable-captcha
as the toggle).
VAPT: File Upload Validation and Security Hardening
There was a problem hiding this comment.
Actionable comments posted: 2
β»οΈ Duplicate comments (3)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java (3)
178-186: Replace brittle blacklist with basename whitelist; preserve expected charsCurrent chain removes hyphen/underscore and duplicates work; it also risks odd filenames. Use a whitelist on the basename and reattach the validated extension.
- kmFileManager.setFileName(kmFileManager.getFileName().replace("`", "").replace("'", "") - .replace("$", "") - .replace("\\", "").replace("/", "").replace("~", "").replace("`", "").replace("!", "") - .replace("@", "").replace("#", "").replace("$", "").replace("%", "").replace("^", "") - .replace("&", "").replace("*", "").replace("(", "").replace(")", "").replace("{", "") - .replace("}", "").replace("[", "").replace("]", "").replace("|", "").replace("\\", "") - .replace(":", "").replace(";", "").replace("-", "").replace("_", "").replace("+", "") - .replace("=", "").replace("\"", "").replace("'", "")); + String originalName = kmFileManager.getFileName(); + String safeBase = FilenameUtils.getBaseName(originalName).replaceAll("[^A-Za-z0-9._-]", ""); + String safeFileName = safeBase + "." + extensionFromName; // extensionFromName already normalized + kmFileManager.setFileName(safeFileName);
144-176: Fix allowlist normalization, blank checks, and logging placeholdersCurrent logic breaks with dots/spaces/case and treats empty extension as valid; logs use string concatenation. Reuse the previously suggested normalization.
Apply:
- if (allowedFileExtensions == null || allowedFileExtensions.trim().isEmpty()) { - throw new IllegalStateException( - "Environment variable 'allowed.file.extensions' is not configured or is empty"); - } - List<String> allowedExtensions = Arrays.asList(allowedFileExtensions.split(",")); - logger.info("Allowed extensions: " + allowedExtensions); + if (allowedFileExtensions == null || allowedFileExtensions.trim().isEmpty()) { + throw new IllegalStateException( + "Configuration property 'allowed.file.extensions' is not configured or empty"); + } + java.util.Set<String> allowedExtensions = Arrays.stream(allowedFileExtensions.split(",")) + .map(String::trim) + .map(s -> s.startsWith(".") ? s.substring(1) : s) + .map(String::toLowerCase) + .filter(s -> !s.isEmpty()) + .collect(java.util.stream.Collectors.toSet()); + if (allowedExtensions.isEmpty()) { + throw new IllegalStateException("Configuration 'allowed.file.extensions' must have at least one value"); + } + logger.info("Allowed extensions: {}", allowedExtensions); @@ - String extensionFromName = FilenameUtils.getExtension(kmFileManager.getFileName()); - logger.info("extensionFromName " + extensionFromName); + String extensionFromName = FilenameUtils.getExtension(kmFileManager.getFileName()); + extensionFromName = extensionFromName == null ? null : extensionFromName.trim().toLowerCase(); + logger.info("extensionFromName {}", extensionFromName); @@ - String extensionFromPayload = kmFileManager.getFileExtension(); - logger.info("extensionPayload " + extensionFromPayload); + String extensionFromPayload = kmFileManager.getFileExtension(); + if (extensionFromPayload != null && extensionFromPayload.startsWith(".")) { + extensionFromPayload = extensionFromPayload.substring(1); + } + extensionFromPayload = extensionFromPayload == null ? null : extensionFromPayload.trim().toLowerCase(); + logger.info("extensionPayload {}", extensionFromPayload); @@ - // Validate extensions - if (extensionFromName == null || extensionFromPayload == null) { + // Validate extensions + if (extensionFromName == null || extensionFromName.isEmpty() + || extensionFromPayload == null || extensionFromPayload.isEmpty()) { throw new IOException("File extension missing"); } @@ - if (!extensionFromName.equalsIgnoreCase(extensionFromPayload)) { + if (!extensionFromName.equals(extensionFromPayload)) { throw new IOException( "File extension mismatch: " + extensionFromName + " vs " + extensionFromPayload); } @@ - if (!allowedExtensions.contains(extensionFromName.toLowerCase())) { + if (!allowedExtensions.contains(extensionFromName)) { throw new IOException("File extension not allowed: " + extensionFromName); }Add if missing:
import java.util.stream.Collectors;
186-197: Harden temp path handling; stream Base64 decode; try-with-resources; structured logsAvoid string path concat, decode streaming to prevent large byte[] allocations, ensure directories exist, and use placeholder logging.
- String tempFilePath = ConfigProperties.getPropertyByName("tempFilePath"); - newFile = new FileOutputStream(tempFilePath + "/" + kmFileManager.getFileName()); - newFile.write(Base64.getDecoder().decode(kmFileManager.getFileContent())); - newFile.flush(); - newFile.close(); - fis = new FileInputStream(tempFilePath + "/" + kmFileManager.getFileName()); - String checksum = DigestUtils.md5DigestAsHex(fis); - fis.close(); - logger.info("File is " + kmFileManager.getFileName()); - logger.info("File size is " + new File(tempFilePath + "/" + kmFileManager.getFileName()).length()); - logger.info("File checksum is " + checksum); - logger.info("File checksum length is " + checksum.length()); + String tempFilePath = ConfigProperties.getPropertyByName("tempFilePath"); + if (tempFilePath == null || tempFilePath.trim().isEmpty()) { + throw new IllegalStateException("Configuration property 'tempFilePath' is not configured or empty"); + } + java.nio.file.Path tempDir = java.nio.file.Paths.get(tempFilePath); + java.nio.file.Files.createDirectories(tempDir); + java.nio.file.Path targetPath = tempDir.resolve(kmFileManager.getFileName()); + try (java.io.InputStream b64In = java.util.Base64.getDecoder() + .wrap(new java.io.ByteArrayInputStream( + kmFileManager.getFileContent().getBytes(java.nio.charset.StandardCharsets.US_ASCII))); + java.io.OutputStream out = java.nio.file.Files.newOutputStream(targetPath)) { + byte[] buffer = new byte[8192]; + int n; + while ((n = b64In.read(buffer)) != -1) { + out.write(buffer, 0, n); + } + } + String checksum; + try (java.io.InputStream in = java.nio.file.Files.newInputStream(targetPath)) { + checksum = org.springframework.util.DigestUtils.md5DigestAsHex(in); + } + logger.info("File is {}", kmFileManager.getFileName()); + logger.info("File size is {}", java.nio.file.Files.size(targetPath)); + logger.info("File checksum is {}", checksum);Note: add imports (outside diff): ByteArrayInputStream, InputStream, OutputStream, Paths, Path, Files, StandardCharsets.
Also consider deleting the temp file after upload (see below).
π§Ή Nitpick comments (5)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java (5)
44-44: Prefer centralized config over scattered @valueConsider a small @ConfigurationProperties class (or dedicated Properties bean) to group KM file-related settings for easier testing and validation.
89-91: Normalize and cache allowed extensions at bean initParsing on each request is error-prone and repetitive. Initialize a normalized Set once and validate against it. Also align error wording with Spring properties (not βenvironment variableβ).
Example (outside diff range):
// fields private Set<String> allowedExtensions; @PostConstruct void initAllowedExtensions() { if (allowedFileExtensions == null || allowedFileExtensions.trim().isEmpty()) { throw new IllegalStateException("Configuration property 'allowed.file.extensions' is not configured or empty"); } allowedExtensions = Arrays.stream(allowedFileExtensions.split(",")) .map(String::trim).map(s -> s.startsWith(".") ? s.substring(1) : s) .map(String::toLowerCase).filter(s -> !s.isEmpty()) .collect(java.util.stream.Collectors.toSet()); }
126-126: Use generics; avoid raw ArrayListThis avoids unchecked warnings.
- kmFileManagers = addKMFile(new ArrayList(Arrays.asList(kmFileManagerArray))); + kmFileManagers = addKMFile(new ArrayList<>(Arrays.asList(kmFileManagerArray)));
220-220: Remove no-op assignmentRedundant self-assignment; safe to delete.
- kmFileManager.setSubCategoryID(kmFileManager.getSubCategoryID());
236-240: Drop manual closes; rely on try-with-resourcesOnce you switch to try-with-resources above, these become unnecessary.
- if(newFile !=null) - newFile.close(); - if(fis !=null) - fis.close();
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π‘ Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
π Files selected for processing (2)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java(7 hunks)src/main/resources/application.properties(1 hunks)
π§ Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.properties
π Additional comments (2)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java (2)
2-6: GPL header updates look goodNo issues spotted with the license header changes.
37-37: Good call importing FilenameUtilsAppropriate use for robust extension extraction.
| kmFileManager.setKmUploadStatus(KM_UPLOADSTATUS_PENDING); | ||
| String version = getFileVersion(kmFileManager); | ||
| kmFileManager.setVersionNo(version); | ||
| String documentPath = kmFileManager.getProviderServiceMapID() + "/"; | ||
| if (kmFileManager.getCategoryID() != null) { | ||
| documentPath += kmFileManager.getCategoryID() + "/"; | ||
| } | ||
| if (kmFileManager.getSubCategoryID() != null) { | ||
| documentPath += kmFileManager.getSubCategoryID() + "/"; | ||
| } | ||
| if (kmFileManager.getVanID() != null) | ||
| documentPath += kmFileManager.getVanID() + "/"; | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Delete temp file after checksum/upload
Temp files will accumulate. After successful upload, remove the local temp file.
Example (outside diff):
// after kmService.createDocument(...)
java.nio.file.Files.deleteIfExists(java.nio.file.Paths.get(tempFilePath).resolve(kmFileManager.getFileName()));If you adopt the targetPath variable from the previous diff, call Files.deleteIfExists(targetPath).
π€ Prompt for AI Agents
In
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java
around lines 199-211, temp files created for checksum/upload are not deleted
causing accumulation; after a successful upload (after the
kmService.createDocument call completes) delete the temp file by calling
java.nio.file.Files.deleteIfExists(...) on the resolved temp file path (or the
targetPath variable if already used), and catch/log any IOException so failures
to delete do not break flow.
| kmFileManager.getFileName()); | ||
| version = "V" + (files.size() + 1); | ||
|
|
There was a problem hiding this comment.
π‘ Verification agent
𧩠Analysis chain
Versioning race risk; ensure monotonic increments under concurrency
Computing V(files.size() + 1) is subject to race conditions. Consider a DB-level unique constraint on (providerServiceMapID, fileName, versionNo) and a βSELECT β¦ FOR UPDATEβ (or a sequence/auto-increment) to generate the next version atomically.
Run to locate repository method and check for locking:
π Script executed:
#!/bin/bash
rg -nP -C3 'getKMFileByFileName\s*\('Length of output: 1950
π Script executed:
#!/bin/bash
find src -type f -name KMFileManager.javaLength of output: 111
π Script executed:
#!/bin/bash
sed -n '1,200p' src/main/java/com/iemr/common/data/kmfilemanager/KMFileManager.javaLength of output: 4614
Ensure atomic versionNo generation to avoid race conditions
Using files.size() + 1 in getFileVersion risks duplicate versionNo under concurrent uploads; enforce a DB unique constraint on (providerServiceMapID, fileName, versionNo) and generate the next version atomically (e.g., sequence, SELECT β¦ FOR UPDATE, or JPA pessimistic lock).
π€ Prompt for AI Agents
In
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java
around line 254, the current getFileVersion implementation computes versionNo
using files.size() + 1 which can produce duplicates under concurrent uploads;
add a database unique constraint on (providerServiceMapID, fileName, versionNo)
and change version generation to be atomicβeither use a dedicated DB sequence
per (providerServiceMapID,fileName), perform a SELECT β¦ FOR UPDATE on the row(s)
holding the current max version within a transactional boundary, or use JPA
pessimistic locking to fetch the current max version and then increment and
persist the new versionβalso ensure the method runs inside a transaction so the
lock is effective and handle unique constraint violations by retrying if
necessary.
* Content disposition modified signature * Coderabbit comments addressed
There was a problem hiding this comment.
Actionable comments posted: 2
π§Ή Nitpick comments (3)
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java (3)
24-25: Imports look good; consider filename-based media type fallback support.If you adopt the filename-based fallback below, add MediaTypeFactory.
+import org.springframework.http.MediaTypeFactory;Also applies to: 30-35, 41-41
73-79: Tighten response headers (nosniff, stronger cache semantics).Small hardening for downloads.
HttpHeaders responseHeaders = new HttpHeaders(); responseHeaders.setContentDisposition( ContentDisposition.attachment().filename(originalName, StandardCharsets.UTF_8).build()); - responseHeaders.setCacheControl(CacheControl.noStore()); + responseHeaders.setCacheControl(CacheControl.noStore().mustRevalidate()); responseHeaders.add(HttpHeaders.PRAGMA, "no-cache"); responseHeaders.setExpires(0); + responseHeaders.add(HttpHeaders.X_CONTENT_TYPE_OPTIONS, "nosniff");
79-85: Avoid catching NPE; add smarter media type detection.Parse provided type when valid; otherwise infer from filename, then default.
- MediaType mediaType; - try { - mediaType = MediaType.parseMediaType(userSignID.getFileType()); - } catch (InvalidMediaTypeException | NullPointerException e) { - mediaType = MediaType.APPLICATION_OCTET_STREAM; - } + MediaType mediaType = null; + String fileType = userSignID.getFileType(); + if (fileType != null && !fileType.isBlank()) { + try { + mediaType = MediaType.parseMediaType(fileType); + } catch (InvalidMediaTypeException ignore) { + // fall through + } + } + if (mediaType == null) { + mediaType = MediaTypeFactory.getMediaType(originalName) + .orElse(MediaType.APPLICATION_OCTET_STREAM); + }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java(2 hunks)
π Additional comments (2)
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java (2)
86-90: LGTM on 404 for empty payload.Good defensive check for missing/empty bytes.
91-92: LGTM on attachment response construction.Headers + content type + length are correctly set.
| String originalName = userSignID.getFileName(); | ||
| if (originalName == null || originalName.isBlank()) { | ||
| originalName = "signature"; | ||
| } | ||
| HttpHeaders responseHeaders = new HttpHeaders(); |
There was a problem hiding this comment.
Prevent NPE when no signature record is found.
userSignID can be null; calling getters will throw NPE. Return 404 early.
- EmployeeSignature userSignID = employeeSignatureServiceImpl.fetchSignature(userID);
+ EmployeeSignature userSignID = employeeSignatureServiceImpl.fetchSignature(userID);
+ if (userSignID == null) {
+ throw new ResponseStatusException(HttpStatus.NOT_FOUND,
+ "Signature not found for userID: " + userID);
+ }π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String originalName = userSignID.getFileName(); | |
| if (originalName == null || originalName.isBlank()) { | |
| originalName = "signature"; | |
| } | |
| HttpHeaders responseHeaders = new HttpHeaders(); | |
| EmployeeSignature userSignID = employeeSignatureServiceImpl.fetchSignature(userID); | |
| if (userSignID == null) { | |
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, | |
| "Signature not found for userID: " + userID); | |
| } | |
| String originalName = userSignID.getFileName(); | |
| if (originalName == null || originalName.isBlank()) { | |
| originalName = "signature"; | |
| } | |
| HttpHeaders responseHeaders = new HttpHeaders(); |
|
|
||
| } catch (Exception e) { | ||
| logger.error("File download for userID failed with exception " + e.getMessage(), e); | ||
| throw new Exception("Error while downloading file. Please contact administrator.."); |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Prefer ResponseStatusException over generic Exception; fix message.
Return an explicit 500 and keep the cause; avoid trailing β..β.
- throw new Exception("Error while downloading file. Please contact administrator..");
+ throw new ResponseStatusException(
+ HttpStatus.INTERNAL_SERVER_ERROR,
+ "Error while downloading file. Please contact administrator.",
+ e
+ );If you adopt this, consider removing throws Exception from the method signature as well to avoid checked exceptions leaking out.
π€ Prompt for AI Agents
In
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java
around line 96, replace throwing a generic Exception with a
ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error while
downloading file. Please contact administrator.", cause) (include the caught
exception as the cause) and remove the trailing β..β; also remove the
now-unneeded throws Exception from the method signature so checked exceptions do
not leak.
* Update JwtUserIdValidationFilter.java * Added new condition for new user status * Status code changes from 5002 to 401 if it is Auauthorized * logger message changed
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (4)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (4)
299-307: Enforce refresh-token rotation: revoke old JTI.Old refresh tokens remain valid until expiry, enabling reuse. Delete the old jti on rotation.
Apply:
// Generate and store a new refresh token (token rotation) String newRefreshToken = jwtUtil.generateRefreshToken(user.getUserName(), userId); String newJti = jwtUtil.getJtiFromToken(newRefreshToken); + // Revoke the old refresh token to prevent reuse + if (jti != null) { + redisTemplate.delete("refresh:" + jti); + } redisTemplate.opsForValue().set( "refresh:" + newJti, userId, jwtUtil.getRefreshTokenExpiration(), TimeUnit.MILLISECONDS );
561-586: Fix NPE on Authorization header; validate token and handle Bearer before extracting userId.authHeader.isEmpty() NPE when header is null. Also, token is not validated and Bearer prefix is ignored.
Apply:
- String authHeader = request.getHeader("Authorization"); - if (authHeader.isEmpty()) { + String authHeader = request.getHeader("Authorization"); + if (authHeader == null || authHeader.isBlank()) { // Try JWT token from header first - String jwtToken = request.getHeader("Jwttoken"); + String jwtToken = request.getHeader("Jwttoken"); + if (jwtToken == null) { + String bearer = request.getHeader("Authorization"); + if (bearer != null && bearer.startsWith("Bearer ")) { + jwtToken = bearer.substring(7); + } + } // If not in header, try cookie if (jwtToken == null) { Cookie[] cookies = request.getCookies(); if (cookies != null) { for (Cookie cookie : cookies) { if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { jwtToken = cookie.getValue(); break; } } } } - if (jwtToken == null) { + if (jwtToken == null || jwtToken.isBlank()) { logger.warn("Authentication failed: no token found in header or cookies."); throw new IEMRException("Authentication failed. Please log in again."); } - // Extract user ID from the JWT token - String userId = jwtUtil.getUserIdFromToken(jwtToken); + // Validate token then extract userId + Claims claims = jwtUtil.validateToken(jwtToken); + if (claims == null) { + logger.warn("Authentication failed: invalid token provided."); + throw new IEMRException("Authentication failed. Please log in again."); + } + String userId = claims.get("userId", String.class);
1170-1188: Do not expose JWT via GET endpoint; undermines HttpOnly.Returning the JWT from cookies to the caller defeats HttpOnly protections and increases XSS/CSRF risk.
Apply one of:
- Remove the endpoint.
- Or restrict and validate: require Authorization Bearer + CSRF token and never echo cookie value.
Minimal safe action (remove):
- @GetMapping("/get-jwt-token") - public ResponseEntity<String> getJwtTokenFromCookie(HttpServletRequest httpRequest) { - // Retrieve the cookie named 'jwtToken' - Cookie[] cookies = httpRequest.getCookies(); - if (cookies != null) { - for (Cookie cookie : cookies) { - if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { - String jwtToken = cookie.getValue(); - // Return the JWT token in the response - return ResponseEntity.ok(jwtToken); - } - } - } - // Return 404 if the token is not found in the cookies - return ResponseEntity.status(HttpStatus.NOT_FOUND).body("JWT token not found"); - }If you must keep it, confirm CORS is locked down and add CSRF validation; otherwise, remove.
I can provide a CSRF/nonce-based design if needed.
140-140: Stop logging plaintext passwords.Avoid logging sensitive data.
Apply:
- logger.info("userAuthenticate request - " + m_User + " " + m_User.getUserName() + " " + m_User.getPassword()); + logger.info("userAuthenticate request - userName={}", m_User != null ? m_User.getUserName() : "null");Also sanitize similar logs elsewhere.
π§Ή Nitpick comments (3)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
83-85: Provide a safe default for enableCaptcha to avoid startup failure.If captcha.enable-captcha is missing in config, the app fails to start. Inject a default.
Apply:
- @Value("${captcha.enable-captcha}") + @Value("${captcha.enable-captcha:false}") private boolean enableCaptcha;
766-771: Typo in operation summary and method naming.βGet security quetionsβ and getSecurityts are misspelled. Rename for clarity.
- @Operation(summary = "Get security quetions") - @RequestMapping(value = "/getsecurityquetions", method = RequestMethod.GET) - public String getSecurityts() { + @Operation(summary = "Get security questions") + @RequestMapping(value = "/getSecurityQuestions", method = RequestMethod.GET) + public String getSecurityQuestions() {Note: Update callers accordingly.
src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java (1)
142-142: Prefer Springβs MediaType constant to avoid javax.ws.rs dependency coupling.Use org.springframework.http.MediaType.APPLICATION_JSON_VALUE.
Apply this diff:
- response.setContentType(MediaType.APPLICATION_JSON); + response.setContentType(org.springframework.http.MediaType.APPLICATION_JSON_VALUE);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(12 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java(1 hunks)src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java(4 hunks)
π§° Additional context used
π§ Learnings (2)
π Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Additional comments (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (4)
173-176: LGTM: Concurrency check gated by withCredentials.This aligns the check with cookie-based sessions. Verify all clients set withCredentials true when using cookies.
288-291: Confirm requirement: allow token refresh for βNewβ users.Treating βNewβ as active changes auth semantics. Ensure this matches policy and doesnβt enable refresh for un-onboarded/blocked users.
951-972: Harden forceLogout: accept tokens from headers, null-safe claims, guard TTL.Current flow ignores headers, risks NPE on claims.isEmpty(), and may add expired tokens with negative TTL.
Apply:
- // Extract token from cookies or headers - String token = getJwtTokenFromCookies(httpRequest); - if (token == null) { - response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - outputResponse.setError(new RuntimeException("No JWT token found in request")); - return outputResponse.toString(); - } + // Extract token from cookies or headers + String token = getJwtTokenFromCookies(httpRequest); + if (token == null) { + token = httpRequest.getHeader(Constants.JWT_TOKEN); + } + if (token == null) { + String authz = httpRequest.getHeader("Authorization"); + if (authz != null && authz.startsWith("Bearer ")) { + token = authz.substring(7); + } + } + if (token == null || token.isBlank()) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + outputResponse.setError(new RuntimeException("No JWT token found in request")); + return outputResponse.toString(); + } // Validate the token: Check if it is expired or in the deny list Claims claims = jwtUtil.validateToken(token); - if (claims.isEmpty() || claims.getExpiration() == null || claims.getId() == null) { // If token is either expired or in the deny list, return 401 Unauthorized + if (claims == null || claims.getExpiration() == null || claims.getId() == null) { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); outputResponse.setError(new RuntimeException("Token is expired or has been logged out")); return outputResponse.toString(); } // Extract the jti (JWT ID) and expiration time from the validated claims String jti = claims.getId(); // jti is in the 'id' field of claims long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims long ttlMillis = expirationTime - System.currentTimeMillis(); - tokenDenylist.addTokenToDenylist(jti, ttlMillis); + if (ttlMillis > 0) { + tokenDenylist.addTokenToDenylist(jti, ttlMillis); + }
983-991: JWT cookie name is consistent Constants.JWT_TOKEN is defined as"Jwttoken"and matches the literal used inaddJwtTokenToCookieandgetJwtTokenFromCookies.src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java (2)
38-38: Import is correct for Boot 3 (jakarta). LGTM.*No issues with using jakarta.servlet.ServletOutputStream.
141-151: Fix CORS on error path: avoid "*", add Vary, and drop manual Content-Length.Using "*" breaks credentialed requests (JWT cookie) and weakens security. Also, let the container handle Content-Length. Set headers before writing body (you already do) and echo only an allowed Origin from config.
Apply this diff:
- response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); // 401 - response.setContentType(MediaType.APPLICATION_JSON); - response.setHeader("Access-Control-Allow-Origin", "*"); - - // Better to use getBytes().length for accurate byte size - byte[] responseBytes = jsonErrorResponse.getBytes(StandardCharsets.UTF_8); - response.setContentLength(responseBytes.length); - - ServletOutputStream out = response.getOutputStream(); - out.write(responseBytes); - out.flush(); + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); // 401 + response.setContentType(MediaType.APPLICATION_JSON); + response.setCharacterEncoding(StandardCharsets.UTF_8.name()); + String allowOrigin = resolveAllowedOrigin(request); + if (allowOrigin != null) { + response.setHeader("Access-Control-Allow-Origin", allowOrigin); + if (!"*".equals(allowOrigin)) { + response.addHeader("Vary", "Origin"); + } + } + try (ServletOutputStream out = response.getOutputStream()) { + out.write(jsonErrorResponse.getBytes(StandardCharsets.UTF_8)); + out.flush(); + }Add these members/imports outside the shown lines:
// imports import org.springframework.beans.factory.annotation.Value; // field @Value("${cors.allowed-origins:}") private String allowedOrigins; // helper private String resolveAllowedOrigin(HttpServletRequest request) { String origin = request.getHeader("Origin"); if (origin == null || origin.isEmpty()) return null; if (allowedOrigins == null || allowedOrigins.isEmpty()) return null; String cfg = allowedOrigins.trim(); if ("*".equals(cfg)) return "*"; for (String allowed : cfg.split(",")) { if (origin.equals(allowed.trim())) return origin; } return null; }#!/bin/bash # Verify CORS config presence/keys and other places setting ACAO rg -nP -C2 'cors\.allowed-origin|cors\.allowed-origins|CorsConfig|CorsFilter|Access-Control-Allow-Origin' --hidden
| String errorMessage = e.getMessage(); | ||
| if (errorMessage == null || errorMessage.trim().isEmpty()) { | ||
| errorMessage = "Unauthorized access or session expired."; | ||
| } | ||
|
|
||
| String jsonErrorResponse = "{" | ||
| + "\"status\": \"Unauthorized\"," | ||
| + "\"statusCode\": 401," | ||
| + "\"errorMessage\": \"" + errorMessage.replace("\"", "\\\"") + "\"" | ||
| + "}"; |
There was a problem hiding this comment.
Do not leak exception details; build JSON safely (use Jackson) and keep a generic client message.
Embedding raw exception messages risks information disclosure and brittle string-escaped JSON. Use ObjectMapper and a fixed client-facing message.
Apply this diff:
- String errorMessage = e.getMessage();
- if (errorMessage == null || errorMessage.trim().isEmpty()) {
- errorMessage = "Unauthorized access or session expired.";
- }
-
- String jsonErrorResponse = "{"
- + "\"status\": \"Unauthorized\","
- + "\"statusCode\": 401,"
- + "\"errorMessage\": \"" + errorMessage.replace("\"", "\\\"") + "\""
- + "}";
+ final String clientMessage = "Unauthorized access or session expired.";
+ String jsonErrorResponse = OBJECT_MAPPER.writeValueAsString(
+ java.util.Map.of(
+ "status", "Unauthorized",
+ "statusCode", 401,
+ "errorMessage", clientMessage
+ )
+ );Add these outside the shown lines:
import com.fasterxml.jackson.databind.ObjectMapper;
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();π€ Prompt for AI Agents
In src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java around
lines 130 to 139, the code currently inserts raw exception messages into a
hand-built JSON string which risks leaking internals and brittle escaping;
replace that block to always return a generic client-facing message (e.g.,
"Unauthorized access or session expired.") and build the JSON using Jackson: add
the suggested import and a private static final ObjectMapper OBJECT_MAPPER = new
ObjectMapper(); outside the shown lines, then create a small Map<String,Object>
(or a simple POJO) with keys status="Unauthorized", statusCode=401,
errorMessage=<fixed message> and call OBJECT_MAPPER.writeValueAsString(map) to
produce the JSON string; ensure you do not include e.getMessage() or other
exception details in the response.
| || path.startsWith(contextPath + "/user/logOutUserFromConcurrentSession") | ||
| || path.startsWith(contextPath + "/user/refreshToken"); |
There was a problem hiding this comment.
Keep refresh/logout skip checks exact to avoid auth bypass
We already have exact matches for /user/logOutUserFromConcurrentSession and /user/refreshToken above. Adding startsWith(...) broadens the skip-list so that any path beginning with those segments bypasses JWT validation (e.g. /user/refreshToken/rotate), reβintroducing the security hole weβve been trying to close. Please drop these two new predicates and keep the stricter equality checks.
- || path.startsWith(contextPath + "/user/logOutUserFromConcurrentSession")
- || path.startsWith(contextPath + "/user/refreshToken");π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| || path.startsWith(contextPath + "/user/logOutUserFromConcurrentSession") | |
| || path.startsWith(contextPath + "/user/refreshToken"); |
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java around
lines 172-173, the two conditions using path.startsWith(contextPath +
"/user/logOutUserFromConcurrentSession") and path.startsWith(contextPath +
"/user/refreshToken") broaden the skip-list and allow auth bypass; remove these
two startsWith predicates and rely on the existing exact equality checks above
(or replace them with explicit equals(...) checks) so only the exact endpoints
are skipped.
fix: missing env added
added missing envs for bengen
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (2)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
271-275: Rotate refresh tokens securely: revoke the old JTI.Token rotation currently issues a new refresh token but keeps the old one valid in Redis. Revoke the old JTI before/when storing the new one.
- // Check revocation using JTI - String jti = claims.getId(); - if (!redisTemplate.hasKey("refresh:" + jti)) { + // Check revocation using JTI (old token) + String oldJti = claims.getId(); + if (!redisTemplate.hasKey("refresh:" + oldJti)) { logger.warn("Token validation failed: refresh token is revoked or not found in store."); return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body("Unauthorized."); } @@ - // Generate and store a new refresh token (token rotation) + // Generate and store a new refresh token (token rotation) + // Revoke the old refresh token + redisTemplate.delete("refresh:" + oldJti); String newRefreshToken = jwtUtil.generateRefreshToken(user.getUserName(), userId); String newJti = jwtUtil.getJtiFromToken(newRefreshToken); redisTemplate.opsForValue().set( "refresh:" + newJti, userId, jwtUtil.getRefreshTokenExpiration(), TimeUnit.MILLISECONDS );Also applies to: 299-307
140-140: Remove plaintext password from logs.Logging credentials is a compliance and security risk. Strip or mask the password.
- logger.info("userAuthenticate request - " + m_User + " " + m_User.getUserName() + " " + m_User.getPassword()); + logger.info("userAuthenticate request - userName={}", m_User.getUserName());- logger.info("userAuthenticate request - " + m_User + " " + m_User.getUserName() + " " + m_User.getPassword()); + logger.info("userAuthenticate request - userName={}", m_User.getUserName());Also applies to: 1121-1121
π§Ή Nitpick comments (2)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
1179-1181: Unify cookie name with Constants.JWT_TOKEN.Avoid string literals for cookie name to prevent drift across endpoints.
- if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { + if (Constants.JWT_TOKEN.equalsIgnoreCase(cookie.getName())) {
380-380: Avoid System.out.println in server code; use logger at debug level.Swap to logger.debug for consistency with other logging.
- System.out.println(mUser); + logger.debug("User mapping for ID={}", mUser != null ? mUser.getUserID() : null);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/main/environment/common_ci.properties(3 hunks)src/main/environment/common_docker.properties(2 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(13 hunks)
π§ Files skipped from review as they are similar to previous changes (1)
- src/main/environment/common_ci.properties
π§° Additional context used
π§ Learnings (3)
π Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Learning: 2025-05-09T12:55:44.224Z
Learnt from: vanitha1822
PR: PSMRI/Common-API#180
File: src/main/environment/common_example.properties:192-192
Timestamp: 2025-05-09T12:55:44.224Z
Learning: The video-call-url property for Jitsi integration is configured in local.properties file, while common_example.properties contains only a placeholder value.
Applied to files:
src/main/environment/common_docker.properties
π Additional comments (7)
src/main/environment/common_example.properties (1)
211-212: Beneficiary ID endpoint config aligns with other environmentsThe added local URL matches the pattern introduced in the CI/Docker property sets, so no issues here.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (6)
83-85: Provide a safe default for enableCaptcha to avoid startup failure.If the property is missing in some envs, app will fail to start. Add a default.
- @Value("${captcha.enable-captcha}") + @Value("${captcha.enable-captcha:false}") private boolean enableCaptcha;
173-176: Confirm intended gating of concurrent-session check with withCredentials.The check only runs when withCredentials == true. Verify this wonβt skip protection for web/non-credential flows unintentionally.
288-291: LGTM: allow refresh for Active or New users.This aligns refresh with the onboarding state while still restricting disabled users.
566-587: Validate the JWT before extracting userId; use a single cookie/header name.Currently calls getUserIdFromToken on an unvalidated token and uses a hardcoded cookie/header name. Validate first and use Constants.JWT_TOKEN.
- if (authHeader.isEmpty()) { - // Try JWT token from header first - String jwtToken = request.getHeader("Jwttoken"); + if (authHeader.isEmpty()) { + // Try JWT token from header first + String jwtToken = request.getHeader(Constants.JWT_TOKEN); // If not in header, try cookie if (jwtToken == null) { Cookie[] cookies = request.getCookies(); if (cookies != null) { for (Cookie cookie : cookies) { - if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { + if (Constants.JWT_TOKEN.equalsIgnoreCase(cookie.getName())) { jwtToken = cookie.getValue(); break; } } } } if (jwtToken == null) { logger.warn("Authentication failed: no token found in header or cookies."); throw new IEMRException("Authentication failed. Please log in again."); } - // Extract user ID from the JWT token - String userId = jwtUtil.getUserIdFromToken(jwtToken); + // Validate token first, then extract userId + Claims claims = jwtUtil.validateToken(jwtToken); + if (claims == null) { + logger.warn("Authentication failed: invalid token provided."); + throw new IEMRException("Authentication failed. Please log in again."); + } + String userId = claims.get("userId", String.class);
952-973: Harden forceLogout: accept tokens from headers, validate null-safely, and guard TTL.
- Accept token from cookies, JWT_TOKEN header, or Authorization: Bearer.
- Avoid NPE by null-checking claims.
- Only add to denylist with positive TTL.
- // Extract token from cookies or headers - String token = getJwtTokenFromCookies(httpRequest); - if (token == null) { - response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - outputResponse.setError(new RuntimeException("No JWT token found in request")); - return outputResponse.toString(); - } + // Extract token from cookies or headers + String token = getJwtTokenFromCookies(httpRequest); + if (token == null) { + token = httpRequest.getHeader(Constants.JWT_TOKEN); + } + if (token == null) { + String authz = httpRequest.getHeader("Authorization"); + if (authz != null && authz.startsWith("Bearer ")) { + token = authz.substring(7); + } + } + if (token == null || token.isBlank()) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + outputResponse.setError(new RuntimeException("No JWT token found in request")); + return outputResponse.toString(); + } // Validate the token: Check if it is expired or in the deny list Claims claims = jwtUtil.validateToken(token); - if (claims.isEmpty() || claims.getExpiration() == null || claims.getId() == null) { // If token is either expired or in the deny list, return 401 Unauthorized + if (claims == null || claims.getExpiration() == null || claims.getId() == null) { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); outputResponse.setError(new RuntimeException("Token is expired or has been logged out")); return outputResponse.toString(); } // Extract the jti (JWT ID) and expiration time from the validated claims String jti = claims.getId(); // jti is in the 'id' field of claims long expirationTime = claims.getExpiration().getTime(); // Use expiration from claims long ttlMillis = expirationTime - System.currentTimeMillis(); - tokenDenylist.addTokenToDenylist(jti, ttlMillis); + if (ttlMillis > 0) { + tokenDenylist.addTokenToDenylist(jti, ttlMillis); + }
2-6: LGTM: license header updates.
| video.recording.path={VIDEO_RECORDING_PATH} | ||
| generateBeneficiaryIDs-api-url={GEN_BENEFICIARY_IDS_API_URL} |
There was a problem hiding this comment.
Fix placeholder syntax for new Docker properties
These entries use {VAR} instead of Springβs ${VAR} syntax, so the deployed container will literally see {VIDEO_RECORDING_PATH} / {GEN_BENEFICIARY_IDS_API_URL} and fail to resolve the configured paths. Please switch to ${...} (and, while touching this block, fix the same issue for jibri.output.path on Line 196).
-video.recording.path={VIDEO_RECORDING_PATH}
-generateBeneficiaryIDs-api-url={GEN_BENEFICIARY_IDS_API_URL}
+video.recording.path=${VIDEO_RECORDING_PATH}
+generateBeneficiaryIDs-api-url=${GEN_BENEFICIARY_IDS_API_URL}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| video.recording.path={VIDEO_RECORDING_PATH} | |
| generateBeneficiaryIDs-api-url={GEN_BENEFICIARY_IDS_API_URL} | |
| video.recording.path=${VIDEO_RECORDING_PATH} | |
| generateBeneficiaryIDs-api-url=${GEN_BENEFICIARY_IDS_API_URL} |
π€ Prompt for AI Agents
In src/main/environment/common_docker.properties around lines 196 to 198, the
new Docker property placeholders use {VAR} instead of Spring's required ${VAR},
causing literal unresolved values; replace jibri.output.path,
video.recording.path, and generateBeneficiaryIDs-api-url values to use
${VIDEO_RECORDING_PATH}, ${GEN_BENEFICIARY_IDS_API_URL} (and the correct env var
for jibri.output.path) respectively so Spring resolves them at runtime.
* fix: add logs * fix: add url * fix: add logs * fix: revert the hardcoded url * removing static field type for env variables * fix: merge with 3.6.0 * fix: merge with 3.6.0 --------- Co-authored-by: 5Amogh <amoghavarsh@navadhiti.com>
There was a problem hiding this comment.
Actionable comments posted: 1
π§Ή Nitpick comments (2)
src/main/java/com/iemr/common/service/beneficiary/IdentityBeneficiaryServiceImpl.java (2)
29-29: Avoid wildcard imports for Gson.Import only the used classes to keep the surface explicit and reduce accidental symbol shadowing.
Apply:
-import com.google.gson.*; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import com.google.gson.JsonSyntaxException;
61-67: Rename config fields to camelCase; UPPER_CASE implies constants.These are mutable instance fields; prefer benGenBaseUrl and benGenApiPath for readability and convention.
Example:
-@Value("${genben-api}") -private String BEN_GEN ; -@Value("${generateBeneficiaryIDs-api-url}") -private String BEN_GEN_API_URL ; +@Value("${genben-api}") +private String benGenBaseUrl; +@Value("${generateBeneficiaryIDs-api-url}") +private String benGenApiPath;Update call sites accordingly (Lines 534β536).
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/main/java/com/iemr/common/service/beneficiary/IdentityBeneficiaryServiceImpl.java(4 hunks)
π§° Additional context used
𧬠Code graph analysis (1)
src/main/java/com/iemr/common/service/beneficiary/IdentityBeneficiaryServiceImpl.java (1)
src/main/java/com/iemr/common/utils/response/OutputResponse.java (1)
OutputResponse(38-244)
π Additional comments (3)
src/main/java/com/iemr/common/service/beneficiary/IdentityBeneficiaryServiceImpl.java (3)
412-425: Defensive checks and JSON parsing guard are solid.Null/empty response handling and JsonSyntaxException guard improve resilience.
407-409: Donβt log full request payloads at INFO; redact PII.Payloads likely contain beneficiary PII. Keep URL at info, move/redact payload to debug.
Apply:
-logger.info("Request Payload: {}", request); +logger.debug("Request Payload: [REDACTED]");
54-58: Property coverage verified across environments All required keys (identity-api-url, identity-1097-api-url, genben-api, generateBeneficiaryIDs-api-url) are present in application.properties, common_ci.properties, common_docker.properties, and common_example.properties.
| logger.info("Request to generate ben IDs: " + request); | ||
| logger.info("Generating ben IDs API URL: " + BEN_GEN + BEN_GEN_API_URL); | ||
| result = httpUtils.post(BEN_GEN + BEN_GEN_API_URL, request, header); | ||
|
|
||
| logger.info("Response from generate ben IDs: " + result); | ||
| OutputResponse identityResponse = inputMapper.gson().fromJson(result, OutputResponse.class); |
There was a problem hiding this comment.
Redact payload/response logs; build URL safely; use parameterized logging.
- PII risk: request/response logged at INFO. Redact and move to DEBUG.
- URL join: BEN_GEN + BEN_GEN_API_URL can double-miss/multi-slash; normalize parts.
- Prefer {} placeholders over string concatenation.
Apply:
-logger.info("Request to generate ben IDs: " + request);
-logger.info("Generating ben IDs API URL: " + BEN_GEN + BEN_GEN_API_URL);
-result = httpUtils.post(BEN_GEN + BEN_GEN_API_URL, request, header);
-logger.info("Response from generate ben IDs: " + result);
+String base = BEN_GEN != null ? BEN_GEN.replaceAll("/+$", "") : "";
+String path = BEN_GEN_API_URL != null ? BEN_GEN_API_URL.replaceAll("^/+", "") : "";
+String apiUrl = base.isEmpty() ? path : (path.isEmpty() ? base : base + "/" + path);
+logger.info("Generating ben IDs API URL: {}", apiUrl);
+logger.debug("Request to generate ben IDs: [REDACTED]");
+result = httpUtils.post(apiUrl, request, header);
+if (result == null || result.isEmpty()) {
+ logger.error("Empty response from generate ben IDs API");
+ throw new IEMRException("No response received from generate ben IDs API");
+}
+logger.debug("Response from generate ben IDs: [REDACTED]");Follow-up: consider wrapping the OutputResponse parsing here in a try/catch for JsonSyntaxException similar to getIdentityResponse for parity.
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info("Request to generate ben IDs: " + request); | |
| logger.info("Generating ben IDs API URL: " + BEN_GEN + BEN_GEN_API_URL); | |
| result = httpUtils.post(BEN_GEN + BEN_GEN_API_URL, request, header); | |
| logger.info("Response from generate ben IDs: " + result); | |
| OutputResponse identityResponse = inputMapper.gson().fromJson(result, OutputResponse.class); | |
| // normalize base URL and path to avoid duplicate or missing slashes | |
| String base = BEN_GEN != null ? BEN_GEN.replaceAll("/+$", "") : ""; | |
| String path = BEN_GEN_API_URL != null ? BEN_GEN_API_URL.replaceAll("^/+", "") : ""; | |
| String apiUrl = base.isEmpty() | |
| ? path | |
| : (path.isEmpty() ? base : base + "/" + path); | |
| // log the final URL at INFO; redact PII in payload/response at DEBUG | |
| logger.info("Generating ben IDs API URL: {}", apiUrl); | |
| logger.debug("Request to generate ben IDs: [REDACTED]"); | |
| result = httpUtils.post(apiUrl, request, header); | |
| if (result == null || result.isEmpty()) { | |
| logger.error("Empty response from generate ben IDs API"); | |
| throw new IEMRException("No response received from generate ben IDs API"); | |
| } | |
| logger.debug("Response from generate ben IDs: [REDACTED]"); | |
| OutputResponse identityResponse = | |
| inputMapper.gson().fromJson(result, OutputResponse.class); |
π€ Prompt for AI Agents
In
src/main/java/com/iemr/common/service/beneficiary/IdentityBeneficiaryServiceImpl.java
around lines 533 to 537, change the logging and URL construction: move
request/response payload logs from INFO to DEBUG and avoid printing raw PII (log
a redacted summary or length instead), replace string concatenation with
parameterized logger calls (logger.debug("Request to generate ben IDs: {}",
redactedRequest) / logger.debug("Response from generate ben IDs: {}",
redactedResponse)), build the endpoint URL using a safe join/normalize method
(trim trailing/leading slashes or use URI/URL builder) instead of BEN_GEN +
BEN_GEN_API_URL to avoid double slashes, and wrap the
inputMapper.gson().fromJson(...) call in a try/catch that catches
JsonSyntaxException and handles/logs it similarly to getIdentityResponse so
parsing failures are handled gracefully.
Moving Signture related changes from 3.5.0 to 3.6.0
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
1171-1189: New endpoint exposes JWT without authentication or validation.The
/get-jwt-tokenendpoint retrieves and returns the JWT cookie value without:
- Authenticating the request
- Validating the token before returning it
- Rate limiting or abuse prevention
This allows anyone to extract JWT tokens from their own cookies, which may facilitate token exfiltration attacks.
Consider one of these approaches:
Option 1: Remove if not needed
- @GetMapping("/get-jwt-token") - public ResponseEntity<String> getJwtTokenFromCookie(HttpServletRequest httpRequest) { - // ... implementation ... - }Option 2: Add authentication and validation
- @GetMapping("/get-jwt-token") + @GetMapping(value = "/get-jwt-token", headers = "Authorization") public ResponseEntity<String> getJwtTokenFromCookie(HttpServletRequest httpRequest) { Cookie[] cookies = httpRequest.getCookies(); if (cookies != null) { for (Cookie cookie : cookies) { if ("Jwttoken".equalsIgnoreCase(cookie.getName())) { String jwtToken = cookie.getValue(); + // Validate before returning + Claims claims = jwtUtil.validateToken(jwtToken); + if (claims == null) { + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body("Invalid token"); + } return ResponseEntity.ok(jwtToken); } } } return ResponseEntity.status(HttpStatus.NOT_FOUND).body("JWT token not found"); }
β»οΈ Duplicate comments (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
566-587: Token extraction proceeds without validation (previously flagged).The JWT token is extracted from headers and cookies but
getUserIdFromTokenis called before validating the token, creating a security risk.This issue was previously identified. Apply the recommended fix:
if (jwtToken == null) { logger.warn("Authentication failed: no token found in header or cookies."); throw new IEMRException("Authentication failed. Please log in again."); } - // Extract user ID from the JWT token - String userId = jwtUtil.getUserIdFromToken(jwtToken); + // Validate token first, then extract user ID + Claims claims = jwtUtil.validateToken(jwtToken); + if (claims == null) { + logger.warn("Authentication failed: invalid token provided."); + throw new IEMRException("Authentication failed. Please log in again."); + } + String userId = claims.get("userId", String.class);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(14 hunks)src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java(1 hunks)src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java(1 hunks)
π§° Additional context used
π§ Learnings (1)
π Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
π Additional comments (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
287-292: Verify intent: allow βNewβ users to refresh tokens
The refreshToken endpoint now accepts users whose M_status is βNewβ as well as βActiveβ; confirm this aligns with your authentication policy or restrict it to only βActiveβ users.
| @Operation(summary = "Get UserId based on userName") | ||
| @GetMapping(value = "/userName/{userName}", produces = MediaType.APPLICATION_JSON, headers = "Authorization") | ||
| public ResponseEntity<?> getUserDetails(@PathVariable("userName") String userName) { | ||
| try { | ||
| List<User> users = iemrAdminUserServiceImpl.getUserIdbyUserName(userName); | ||
| if (users.isEmpty()) { | ||
| return new ResponseEntity<>(Map.of("error", "UserName Not Found"), HttpStatus.NOT_FOUND); | ||
| } | ||
| User user = users.get(0); | ||
| return new ResponseEntity<>(Map.of("userName", user.getUserName(), "userId", user.getUserID()), HttpStatus.OK); | ||
| } catch (Exception e) { | ||
| return new ResponseEntity<>(Map.of("error", "Internal server error"), HttpStatus.INTERNAL_SERVER_ERROR); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Input validation missing for username parameter.
The new /userName/{userName} endpoint accepts the userName path variable without validation for:
- Null/empty values (Spring may allow empty strings)
- SQL injection patterns
- Length constraints
- Special characters
Additionally, the error response leaks implementation details ("Internal server error" suggests unhandled exceptions).
Apply this diff:
@Operation(summary = "Get UserId based on userName")
@GetMapping(value = "/userName/{userName}", produces = MediaType.APPLICATION_JSON, headers = "Authorization")
public ResponseEntity<?> getUserDetails(@PathVariable("userName") String userName) {
try {
+ // Validate input
+ if (userName == null || userName.trim().isEmpty()) {
+ return new ResponseEntity<>(Map.of("error", "Username is required"), HttpStatus.BAD_REQUEST);
+ }
+ if (userName.length() > 255) {
+ return new ResponseEntity<>(Map.of("error", "Username too long"), HttpStatus.BAD_REQUEST);
+ }
+
List<User> users = iemrAdminUserServiceImpl.getUserIdbyUserName(userName);
if (users.isEmpty()) {
return new ResponseEntity<>(Map.of("error", "UserName Not Found"), HttpStatus.NOT_FOUND);
}
User user = users.get(0);
return new ResponseEntity<>(Map.of("userName", user.getUserName(), "userId", user.getUserID()), HttpStatus.OK);
} catch (Exception e) {
+ logger.error("Error fetching user details for userName: {}", userName, e);
return new ResponseEntity<>(Map.of("error", "Internal server error"), HttpStatus.INTERNAL_SERVER_ERROR);
}
}π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Operation(summary = "Get UserId based on userName") | |
| @GetMapping(value = "/userName/{userName}", produces = MediaType.APPLICATION_JSON, headers = "Authorization") | |
| public ResponseEntity<?> getUserDetails(@PathVariable("userName") String userName) { | |
| try { | |
| List<User> users = iemrAdminUserServiceImpl.getUserIdbyUserName(userName); | |
| if (users.isEmpty()) { | |
| return new ResponseEntity<>(Map.of("error", "UserName Not Found"), HttpStatus.NOT_FOUND); | |
| } | |
| User user = users.get(0); | |
| return new ResponseEntity<>(Map.of("userName", user.getUserName(), "userId", user.getUserID()), HttpStatus.OK); | |
| } catch (Exception e) { | |
| return new ResponseEntity<>(Map.of("error", "Internal server error"), HttpStatus.INTERNAL_SERVER_ERROR); | |
| } | |
| } | |
| @Operation(summary = "Get UserId based on userName") | |
| @GetMapping(value = "/userName/{userName}", produces = MediaType.APPLICATION_JSON, headers = "Authorization") | |
| public ResponseEntity<?> getUserDetails(@PathVariable("userName") String userName) { | |
| try { | |
| // Validate input | |
| if (userName == null || userName.trim().isEmpty()) { | |
| return new ResponseEntity<>(Map.of("error", "Username is required"), HttpStatus.BAD_REQUEST); | |
| } | |
| if (userName.length() > 255) { | |
| return new ResponseEntity<>(Map.of("error", "Username too long"), HttpStatus.BAD_REQUEST); | |
| } | |
| List<User> users = iemrAdminUserServiceImpl.getUserIdbyUserName(userName); | |
| if (users.isEmpty()) { | |
| return new ResponseEntity<>(Map.of("error", "UserName Not Found"), HttpStatus.NOT_FOUND); | |
| } | |
| User user = users.get(0); | |
| return new ResponseEntity<>(Map.of("userName", user.getUserName(), "userId", user.getUserID()), HttpStatus.OK); | |
| } catch (Exception e) { | |
| logger.error("Error fetching user details for userName: {}", userName, e); | |
| return new ResponseEntity<>(Map.of("error", "Internal server error"), HttpStatus.INTERNAL_SERVER_ERROR); | |
| } | |
| } |
| List<User> getUserIdbyUserName(String userName) throws IEMRException; | ||
|
|
There was a problem hiding this comment.
Method naming violates Java conventions; return type suggests username isn't unique.
The method name getUserIdbyUserName should follow camelCase: getUserByUserName or getUserIdByUserName. Additionally, returning List<User> implies multiple users can share a username, which conflicts with typical authentication constraints where usernames are unique identifiers.
Apply this diff:
- List<User> getUserIdbyUserName(String userName) throws IEMRException;
+ User getUserByUserName(String userName) throws IEMRException;Note: If username uniqueness is guaranteed, return a single User (or Optional<User>). If not, verify whether userExitsCheck (line 51) already provides this functionality.
π Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| List<User> getUserIdbyUserName(String userName) throws IEMRException; | |
| User getUserByUserName(String userName) throws IEMRException; |
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java around
lines 124-125, the method name and return type are problematic: rename
getUserIdbyUserName to follow camelCase and meaning (e.g., getUserByUserName or
getUserIdByUserName) and change the return type to a single User or
Optional<User> if usernames are unique; if usernames truly can be non-unique,
keep List<User> but document that fact. Update the interface signature
accordingly, then update all implementing classes and call sites to the new
method name and return type; also verify whether the existing userExitsCheck
(around line 51) already covers this behavior and reconcile duplication.
| @Override | ||
| public List<User> getUserIdbyUserName(String userName) { | ||
|
|
||
| return iEMRUserRepositoryCustom.findByUserName(userName); | ||
| } |
There was a problem hiding this comment.
π οΈ Refactor suggestion | π Major
Duplicate functionality: identical to userExitsCheck at line 439.
This method duplicates userExitsCheck(String userName) which already delegates to iEMRUserRepositoryCustom.findByUserName(userName). Additionally, no validation is performed on the userName parameter before the repository call.
Consider one of these approaches:
Option 1: Remove duplicate and use existing method
- @Override
- public List<User> getUserIdbyUserName(String userName) {
-
- return iEMRUserRepositoryCustom.findByUserName(userName);
- }
+ // Removed: use userExitsCheck() insteadOption 2: Add validation if keeping this method
@Override
- public List<User> getUserIdbyUserName(String userName) {
-
+ public List<User> getUserByUserName(String userName) throws IEMRException {
+ if (userName == null || userName.trim().isEmpty()) {
+ throw new IEMRException("Username cannot be null or empty");
+ }
return iEMRUserRepositoryCustom.findByUserName(userName);
}Committable suggestion skipped: line range outside the PR's diff.
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
around lines 1222-1226, getUserIdbyUserName duplicates userExitsCheck(String
userName) at ~line 439 and lacks validation; either remove this method and
update all callers to use userExitsCheck, or if you must keep it, add input
validation (null/empty/trim) and throw or return an empty list for invalid
input, and ensure both methods delegate to a single private helper that calls
iEMRUserRepositoryCustom.findByUserName(userName) to avoid duplication.
π Description
JIRA ID: FLW
1.Set up Firebase in the project.
2.Implemented OTP Consent functionality.
3.Worked on related code integration and configuration.
4. dynamic form api.
β Type of Change
βΉοΈ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Configuration
Improvements
Chores