From cd9c36e00e7d2e9fa8d6b415dc4be7904ea41934 Mon Sep 17 00:00:00 2001 From: neel1996 <47709856+neel1996@users.noreply.github.com> Date: Wed, 13 Jul 2022 20:41:26 +0530 Subject: [PATCH] API - Fix cors bug in http interceptor - Resolves bug #160 - CORS related config can be set from environment variables now for flexibility - Added logic to handle preflight requests in HTTP interceptor - Removed OPTIONS http method from the DB table --- .../interceptor/DefaultHttpInterceptor.java | 50 +++++++++++++++++-- .../security/DefaultAuthConfiguration.java | 35 ++++++------- .../src/main/resources/application-dev.yml | 5 +- .../src/main/resources/application.yml | 6 ++- .../0033_delete_options_from_http_methods.xml | 17 +++++++ .../main/resources/db/liquibase-changelog.xml | 1 + .../repository/HttpMethodsRepositoryTest.java | 4 +- .../src/test/resources/application.yml | 4 ++ 8 files changed, 95 insertions(+), 27 deletions(-) create mode 100644 mimock-backend/src/main/resources/db/changelogs/0033_delete_options_from_http_methods.xml diff --git a/mimock-backend/src/main/java/com/arbindo/mimock/interceptor/DefaultHttpInterceptor.java b/mimock-backend/src/main/java/com/arbindo/mimock/interceptor/DefaultHttpInterceptor.java index 910a2d98..151fcb7c 100644 --- a/mimock-backend/src/main/java/com/arbindo/mimock/interceptor/DefaultHttpInterceptor.java +++ b/mimock-backend/src/main/java/com/arbindo/mimock/interceptor/DefaultHttpInterceptor.java @@ -6,6 +6,8 @@ import lombok.extern.log4j.Log4j2; import org.apache.logging.log4j.Level; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; import org.springframework.web.servlet.HandlerInterceptor; @@ -15,6 +17,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintWriter; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -27,6 +30,18 @@ public class DefaultHttpInterceptor implements HandlerInterceptor { private final GenericMockRequestController genericMockRequestController; private final WriterCollection writerCollection; + @Value("#{'${app.security.cors-config.allowed-origins}'.split(',')}") + private List corsAllowedOrigins; + + @Value("#{'${app.security.cors-config.allowed-methods}'.split(',')}") + private List corsAllowedMethods; + + @Value("#{'${app.security.cors-config.allowed-headers}'.split(',')}") + private List corsAllowedHeaders; + + @Value("#{'${app.security.cors-config.exposed-headers}'.split(',')}") + private List corsExposedHeaders; + @Autowired DefaultHttpInterceptor(GenericMockRequestController genericMockRequestController, WriterCollection writerCollection) { this.genericMockRequestController = genericMockRequestController; @@ -39,9 +54,30 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons log.log(Level.INFO, "Intercepting request for " + path); + if (request.getMethod().equals(HttpMethod.OPTIONS.toString())) { + log.log(Level.WARN, "Method is {}. Considering as preflight request", request.getMethod()); + handleAsPreflightRequest(request, response); + return false; + } + return checkAndHandleAsMockRequest(request, response, path); } + private String getOrigin(HttpServletRequest request) { + String requestOrigin = request.getHeader("Origin"); + for (String origin : corsAllowedOrigins) { + if (origin.equals(requestOrigin)) { + return origin; + } + } + return ""; + } + + private void handleAsPreflightRequest(HttpServletRequest request, HttpServletResponse response) { + setPreflightHeaders(response, getOrigin(request)); + response.setStatus(HttpStatus.OK.value()); + } + private boolean checkAndHandleAsMockRequest(HttpServletRequest request, HttpServletResponse response, String path) { log.log(Level.INFO, "Routing intercepted request to mock controller"); Optional domainModelForMock = genericMockRequestController.serveRequest( @@ -57,13 +93,13 @@ private boolean checkAndHandleAsMockRequest(HttpServletRequest request, HttpServ DomainModelForMock matchingMock = domainModelForMock.get(); setStatusAndContentType(response, matchingMock); - setResponseHeaders(response, matchingMock); + setResponseHeaders(request, response, matchingMock); writeResponse(response, matchingMock); return false; } - private void setResponseHeaders(HttpServletResponse response, DomainModelForMock matchingMock) { + private void setResponseHeaders(HttpServletRequest request, HttpServletResponse response, DomainModelForMock matchingMock) { log.log(Level.INFO, "Writing response headers"); if (matchingMock.getResponseHeaders() != null && !matchingMock.getResponseHeaders().isEmpty()) { for (Map.Entry item : matchingMock.getResponseHeaders().entrySet()) { @@ -71,11 +107,19 @@ private void setResponseHeaders(HttpServletResponse response, DomainModelForMock response.setHeader(key, item.getValue().toString()); } } + setPreflightHeaders(response, getOrigin(request)); + } + + private void setPreflightHeaders(HttpServletResponse response, String origin) { + response.setHeader("Access-Control-Allow-Origin", origin); + response.setHeader("Access-Control-Allow-Headers", "*"); + response.setHeader("Access-Control-Allow-Methods", String.join("", corsAllowedMethods)); + response.setHeader("Access-Control-Expose-Headers", String.join("", corsExposedHeaders)); } private void writeResponse(HttpServletResponse response, DomainModelForMock matchingMock) { try { - log.log(Level.INFO, "Writing response the matching mock"); + log.log(Level.INFO, "Writing response for the matching mock"); writerCollection.getWriterFor(matchingMock.getTypeOfResponse()).write(matchingMock, response); } catch (IOException e) { log.log(Level.ERROR, "Response writer exited with a failure : {}", e.getMessage()); diff --git a/mimock-backend/src/main/java/com/arbindo/mimock/security/DefaultAuthConfiguration.java b/mimock-backend/src/main/java/com/arbindo/mimock/security/DefaultAuthConfiguration.java index 7cd1fdd3..ecca44fd 100644 --- a/mimock-backend/src/main/java/com/arbindo/mimock/security/DefaultAuthConfiguration.java +++ b/mimock-backend/src/main/java/com/arbindo/mimock/security/DefaultAuthConfiguration.java @@ -32,6 +32,15 @@ public class DefaultAuthConfiguration extends WebSecurityConfigurerAdapter { @Value("#{'${app.security.cors-config.allowed-origins}'.split(',')}") private List corsAllowedOrigins; + @Value("#{'${app.security.cors-config.allowed-methods}'.split(',')}") + private List corsAllowedMethods; + + @Value("#{'${app.security.cors-config.allowed-headers}'.split(',')}") + private List corsAllowedHeaders; + + @Value("#{'${app.security.cors-config.exposed-headers}'.split(',')}") + private List corsExposedHeaders; + @Override protected void configure(AuthenticationManagerBuilder auth) throws Exception { auth.userDetailsService(userDetailsService); @@ -62,7 +71,7 @@ protected void configure(HttpSecurity http) throws Exception { .and().sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS); setupCSRF(http, apiPath + wildCardPath); - setupCorsConfig(http); + setupCorsConfig(http, apiPath + wildCardPath); http.headers().frameOptions().sameOrigin(); http.addFilterBefore(jwtRequestFilter, UsernamePasswordAuthenticationFilter.class); @@ -75,29 +84,15 @@ private void setupCSRF(HttpSecurity http, String apiPath) throws Exception { .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())); } - private void setupCorsConfig(HttpSecurity http) throws Exception { - http.cors(cors -> { + private void setupCorsConfig(HttpSecurity http, String apiPath) throws Exception { + http.antMatcher(apiPath).cors(cors -> { CorsConfigurationSource cs = resources -> { CorsConfiguration corsConfiguration = new CorsConfiguration(); corsConfiguration.setAllowedOrigins(corsAllowedOrigins); - corsConfiguration.setAllowedMethods(List.of("POST", "GET", "PUT", "DELETE", "OPTIONS")); - corsConfiguration.setAllowedHeaders(List.of( - "Authorization", - "Content-Type", - "X-Requested-With", - "Accept", - "X-XSRF-TOKEN")); + corsConfiguration.setAllowedMethods(corsAllowedMethods); + corsConfiguration.setAllowedHeaders(corsAllowedHeaders); + corsConfiguration.setExposedHeaders(corsExposedHeaders); corsConfiguration.setAllowCredentials(true); - corsConfiguration.setExposedHeaders(List.of( - "Cache-Control", - "Content-Language", - "Content-Length", - "Content-Type", - "Content-Disposition", - "Expires", - "Last-Modified", - "Pragma" - )); return corsConfiguration; }; diff --git a/mimock-backend/src/main/resources/application-dev.yml b/mimock-backend/src/main/resources/application-dev.yml index 50b9ce35..8675047f 100644 --- a/mimock-backend/src/main/resources/application-dev.yml +++ b/mimock-backend/src/main/resources/application-dev.yml @@ -42,7 +42,7 @@ spring: jpa: hibernate: ddl-auto: none - show-sql: true + show-sql: false properties: hibernate: dialect: org.hibernate.dialect.PostgreSQLDialect @@ -74,6 +74,9 @@ app: jwt-secret-key: B4982E53863B4DA7FDA4E1236C4BAF8A # Only for development. Do not use this in production. cors-config: allowed-origins: http://localhost:3000,http://localhost:3001 + allowed-methods: POST,GET,PUT,DELETE,OPTIONS + allowed-headers: Authorization,Content-Type,X-Requested-With,Accept,X-XSRF-TOKEN + exposed-headers: Cache-Control,Content-Language,Content-Length,Content-Type,Content-Disposition,Expires,Last-Modified,Pragma springdoc: swagger-ui: diff --git a/mimock-backend/src/main/resources/application.yml b/mimock-backend/src/main/resources/application.yml index cee80c6f..ccc114ca 100644 --- a/mimock-backend/src/main/resources/application.yml +++ b/mimock-backend/src/main/resources/application.yml @@ -66,7 +66,11 @@ app: jwt-expiry-in-seconds: ${MIMOCK_JWT_EXPIRY_IN_SECONDS} jwt-secret-key: ${MIMOCK_JWT_SECRET} # Only for testing. Do not use this in production. cors-config: - allowed-origins: http://localhost:3000,http://localhost:3001 + allowed-origins: ${MIMOCK_CORS_ORIGINS} + allowed-methods: ${MIMOCK_CORS_METHODS} + allowed-headers: ${MIMOCK_CORS_ALLOWED_HEADERS} + exposed-headers: ${MIMOCK_CORS_EXPOSED_HEADERS} + springdoc: swagger-ui: diff --git a/mimock-backend/src/main/resources/db/changelogs/0033_delete_options_from_http_methods.xml b/mimock-backend/src/main/resources/db/changelogs/0033_delete_options_from_http_methods.xml new file mode 100644 index 00000000..55d08758 --- /dev/null +++ b/mimock-backend/src/main/resources/db/changelogs/0033_delete_options_from_http_methods.xml @@ -0,0 +1,17 @@ + + + + + DELETE + FROM http_methods + WHERE method = 'OPTIONS'; + + + + INSERT INTO http_methods(method, created_at) VALUES ('OPTIONS', current_timestamp); + + + + \ No newline at end of file diff --git a/mimock-backend/src/main/resources/db/liquibase-changelog.xml b/mimock-backend/src/main/resources/db/liquibase-changelog.xml index ef339d1a..ee2379ca 100644 --- a/mimock-backend/src/main/resources/db/liquibase-changelog.xml +++ b/mimock-backend/src/main/resources/db/liquibase-changelog.xml @@ -36,4 +36,5 @@ + diff --git a/mimock-backend/src/test/java/com/arbindo/mimock/repository/HttpMethodsRepositoryTest.java b/mimock-backend/src/test/java/com/arbindo/mimock/repository/HttpMethodsRepositoryTest.java index b011c77a..09078c14 100644 --- a/mimock-backend/src/test/java/com/arbindo/mimock/repository/HttpMethodsRepositoryTest.java +++ b/mimock-backend/src/test/java/com/arbindo/mimock/repository/HttpMethodsRepositoryTest.java @@ -17,7 +17,7 @@ class HttpMethodsRepositoryTest { HttpMethodsRepository httpMethodsRepository; @ParameterizedTest - @ValueSource(strings = {"GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH"}) + @ValueSource(strings = {"GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "TRACE", "PATCH"}) void shouldReturnHttpMethodForValidMethod(String method) { // Act HttpMethod httpMethod = httpMethodsRepository.findByMethod(method); @@ -27,7 +27,7 @@ void shouldReturnHttpMethodForValidMethod(String method) { } @ParameterizedTest - @ValueSource(strings = {"TEST", "RANDOM", "EXEC", "123X"}) + @ValueSource(strings = {"TEST", "RANDOM", "EXEC", "123X", "OPTIONS"}) void shouldReturnNullForInvalidMethod(String method) { // Act HttpMethod httpMethod = httpMethodsRepository.findByMethod(method); diff --git a/mimock-backend/src/test/resources/application.yml b/mimock-backend/src/test/resources/application.yml index 5f86fda3..0453ef30 100644 --- a/mimock-backend/src/test/resources/application.yml +++ b/mimock-backend/src/test/resources/application.yml @@ -48,5 +48,9 @@ app: jwt-secret-key: C4BE6B45CBBD4CBADFE5E22F4BCDBAF8 # Only for testing. Do not use this in production. cors-config: allowed-origins: http://localhost:3000,http://localhost:3001 + allowed-methods: POST,GET,PUT,DELETE,OPTIONS + allowed-headers: Authorization,Content-Type,X-Requested-With,Accept,X-XSRF-TOKEN + exposed-headers: Cache-Control,Content-Language,Content-Length,Content-Type,Content-Disposition,Expires,Last-Modified,Pragma + flush-bin-cron-expression: "*/4 * * * * *" \ No newline at end of file