-
Notifications
You must be signed in to change notification settings - Fork 26
fix: amm-1927 res based for only allowed origins #61
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,24 +37,52 @@ 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("Request Method: {}", method); | ||
| logger.debug("Request URI: {}", uri); | ||
| logger.debug("Allowed Origins Configured: {}", allowedOrigins); | ||
|
|
||
| if ("OPTIONS".equalsIgnoreCase(method)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 and revert? |
||
| if (origin == null) { | ||
| 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; | ||
| } | ||
| } | ||
|
|
||
| // Set CORS headers and handle OPTIONS request only if origin is valid and | ||
| // allowed | ||
| 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-Credentials", "true"); | ||
| addCorsHeaders(response, origin); | ||
| logger.info("Origin Validated | Origin: {} | Method: {} | URI: {}", origin, method, uri); | ||
|
|
||
| if ("OPTIONS".equalsIgnoreCase(method)) { | ||
| // OPTIONS (preflight) - respond with full allowed methods | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| return; | ||
| } | ||
| } 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; | ||
| if ("OPTIONS".equalsIgnoreCase(method)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please check
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please check |
||
| response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed for OPTIONS request"); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| String path = request.getRequestURI(); | ||
|
|
@@ -180,4 +208,13 @@ private void clearUserIdCookie(HttpServletResponse response) { | |
| cookie.setMaxAge(0); // Invalidate the cookie | ||
| response.addCookie(cookie); | ||
| } | ||
|
|
||
| private void addCorsHeaders(HttpServletResponse response, String 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"); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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 and revert?