Skip to content

fix: amm-1927 res headers based on origin via allowed cors #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
5Amogh merged 4 commits into from
Nov 20, 2025
Merged

fix: amm-1927 res headers based on origin via allowed cors #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e5aca1
fix: amm-1927 res headers based on origin via allowed cors
5Amogh Nov 17, 2025
c8f1b33
fix: amm-1927 coderabbit comments resolved
5Amogh Nov 17, 2025
8b431b6
localhost regex added
5Amogh Nov 18, 2025
ce2a0b0
Update regex pattern for localhost in interceptor
5Amogh Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,46 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
HttpServletResponse response = (HttpServletResponse) servletResponse;

String origin = request.getHeader("Origin");
String method = request.getMethod();
String uri = request.getRequestURI();

logger.debug("Incoming Origin: {}", origin);
logger.debug("Allowed Origins Configured: {}", allowedOrigins);
if ("OPTIONS".equalsIgnoreCase(method)) {
if (origin == null) {
Copy link
Member

@vishwab1 vishwab1 Nov 19, 2025

Choose a reason for hiding this comment

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

The current implementation returns 200 OK for OPTIONS requests, which is the correct behavior for CORS preflight requests. Could you please check it

logger.warn("BLOCKED - OPTIONS request without Origin header | Method: {} | URI: {}", method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "OPTIONS request requires Origin header");
return;
}
if (!isOriginAllowed(origin)) {
logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed");
return;
}
} else {
// For non-OPTIONS requests, validate origin if present
if (origin != null && !isOriginAllowed(origin)) {
logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed");
return;
}
}

String path = request.getRequestURI();
String contextPath = request.getContextPath();
Comment on lines +65 to +66
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused variables.

The path and contextPath variables declared here appear to be unused in the immediate context. The path variable is redeclared at line 92 where it's actually used.

Apply this diff:

-		String path = request.getRequestURI();
-		String contextPath = request.getContextPath();
-

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java around lines
65-66, the local variables "path" and "contextPath" are declared but never used
(the "path" is redeclared later at line 92); remove these two unused variable
declarations to avoid redundancy and dead code, keeping the later correct "path"
declaration at line 92 intact.

Copy link
Member Author

@5Amogh 5Amogh Nov 17, 2025

Choose a reason for hiding this comment

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

tolerable for readbility

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

@5Amogh, understood! I'll keep the variables as they are for readability. Thanks for the clarification.

🧠 Learnings used 
Learnt from: ravishanigarapu
Repo: PSMRI/TM-API PR: 79
File: src/main/java/com/iemr/tm/utils/UserAgentContext.java:3-18
Timestamp: 2025-05-22T09:27:49.422Z
Learning: JwtUserIdValidationFilter.java already includes a try-finally block to properly clear UserAgentContext's ThreadLocal resources after use to prevent memory leaks.


if (origin != null && isOriginAllowed(origin)) {
response.setHeader("Access-Control-Allow-Origin", origin);
response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
response.setHeader("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, Jwttoken");
response.setHeader("Vary", "Origin");
response.setHeader("Access-Control-Allow-Origin", origin); // Never use wildcard
response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS");
response.setHeader("Access-Control-Allow-Headers",
"Authorization, Content-Type, Accept, Jwttoken, serverAuthorization, ServerAuthorization, serverauthorization, Serverauthorization");
response.setHeader("Access-Control-Allow-Credentials", "true");
response.setHeader("Access-Control-Max-Age", "3600");
logger.info("Origin Validated | Origin: {} | Method: {} | URI: {}", origin, method, uri);
} else {
logger.warn("Origin [{}] is NOT allowed. CORS headers NOT added.", origin);
}

if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
logger.info("OPTIONS request - skipping JWT validation");
response.setStatus(HttpServletResponse.SC_OK);
return;
}

String path = request.getRequestURI();
String contextPath = request.getContextPath();
logger.info("JwtUserIdValidationFilter invoked for path: " + path);

// Log cookies for debugging
Expand Down Expand Up @@ -142,7 +160,7 @@ private boolean isOriginAllowed(String origin) {
String regex = pattern
.replace(".", "\\.")
.replace("*", ".*")
.replace("http://localhost:.*", "http://localhost:\\d+"); // special case for wildcard port
Copy link
Member

@drtechie drtechie Nov 17, 2025

Choose a reason for hiding this comment

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

@vishwab1 so should this remain as is?

Copy link
Member

@vishwab1 vishwab1 Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@drtechie @5Amogh yes we can keep as it is

.replace("http://localhost:.*", "http://localhost:\\d+");

boolean matched = origin.matches(regex);
return matched;
Expand Down
39 changes: 37 additions & 2 deletions src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@
*/
package com.iemr.tm.utils.http;

import java.util.Arrays;

import javax.ws.rs.core.MediaType;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.HandlerInterceptor;
Expand All @@ -39,6 +42,9 @@
@Component
public class HTTPRequestInterceptor implements HandlerInterceptor {
Logger logger = LoggerFactory.getLogger(this.getClass().getSimpleName());

@Value("${cors.allowed-origins}")
private String allowedOrigins;

private SessionObject sessionObject;

Expand Down Expand Up @@ -95,7 +101,13 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
response.getOutputStream().print(output.toString());
response.setContentType(MediaType.APPLICATION_JSON);
response.setContentLength(output.toString().length());
response.setHeader("Access-Control-Allow-Origin", "*");
String origin = request.getHeader("Origin");
if (origin != null && isOriginAllowed(origin)) {
response.setHeader("Access-Control-Allow-Origin", origin);
response.setHeader("Access-Control-Allow-Credentials", "true");
} else if (origin != null) {
logger.warn("CORS headers NOT added for error response | Unauthorized origin: {}", origin);
}
status = false;
}
}
Expand Down Expand Up @@ -126,4 +138,27 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp
throws Exception {
logger.debug("In afterCompletion Request Completed");
}
}

/**
* Check if the given origin is allowed based on configured allowedOrigins.
* Uses the same logic as JwtUserIdValidationFilter for consistency.
*
* @param origin The origin to validate
* @return true if origin is allowed, false otherwise
*/
private boolean isOriginAllowed(String origin) {
if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
return false;
}

return Arrays.stream(allowedOrigins.split(","))
.map(String::trim)
.anyMatch(pattern -> {
String regex = pattern
.replace(".", "\\.")
.replace("*", ".*")
.replace("http://localhost:.*", "http://localhost:\\d+");
return origin.matches(regex);
});
}
Comment on lines +149 to +163
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated origin validation to a shared utility.

The isOriginAllowed method is duplicated in both HTTPRequestInterceptor and JwtUserIdValidationFilter (line 163-179 in that file). This violates DRY principles and increases maintenance burden.

Create a shared utility class for CORS validation:

package com.iemr.tm.utils.cors;

import java.util.Arrays;

public class CorsUtil {
    
    /**
     * Check if the given origin is allowed based on configured allowedOrigins.
     * 
     * @param origin The origin to validate
     * @param allowedOrigins Comma-separated list of allowed origin patterns
     * @return true if origin is allowed, false otherwise
     */
    public static boolean isOriginAllowed(String origin, String allowedOrigins) {
        if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
            return false;
        }

        return Arrays.stream(allowedOrigins.split(","))
                .map(String::trim)
                .anyMatch(pattern -> {
                    String regex = pattern
                            .replace(".", "\\.")
                            .replace("*", ".*");
                    return origin.matches(regex);
                });
    }
}

Then update both classes to use CorsUtil.isOriginAllowed(origin, allowedOrigins).

🤖 Prompt for AI Agents 
In src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java around lines
149 to 162, the origin validation logic is duplicated in
JwtUserIdValidationFilter; extract this into a new shared utility class (e.g.,
package com.iemr.tm.utils.cors.CorsUtil) with a static method
isOriginAllowed(String origin, String allowedOrigins) that implements the same
null/empty checks and pattern matching, then replace the local isOriginAllowed
implementations in both HTTPRequestInterceptor and JwtUserIdValidationFilter to
call CorsUtil.isOriginAllowed(origin, allowedOrigins); ensure imports are added
and remove the duplicated private methods.

}
Loading