From e935018b0c4dec64c09ddeb90257fef138ac243f Mon Sep 17 00:00:00 2001 From: sdeleuze Date: Mon, 15 Jan 2018 15:55:23 +0100 Subject: [PATCH] Fix SockJs CorsConfiguration for forbidden origins After this commit, AbstractSockJsService uses the configured allowed origins when generating the CorsConfiguration instead of "*". As a consequence, forbidden origin requests still result in a 403 response but now with no CORS headers in order to improve consistency between the status code and the headers. Issue: SPR-16304 --- .../sockjs/support/AbstractSockJsService.java | 5 +++-- .../sockjs/support/SockJsServiceTests.java | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java index 761671febca..e56169b7448 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.web.socket.sockjs.support; import java.io.IOException; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -492,7 +493,7 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { if (!this.suppressCors && CorsUtils.isCorsRequest(request)) { CorsConfiguration config = new CorsConfiguration(); - config.addAllowedOrigin("*"); + config.setAllowedOrigins(new ArrayList(this.allowedOrigins)); config.addAllowedMethod("*"); config.setAllowCredentials(true); config.setMaxAge(ONE_YEAR); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java index 7565c178948..853b9603c01 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import org.springframework.http.server.ServerHttpResponse; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; +import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.socket.AbstractHttpRequestTests; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.sockjs.SockJsException; @@ -172,7 +173,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test // SPR-12226 and SPR-12660 - public void handleInfoOptionsWithOrigin() throws Exception { + public void handleInfoOptionsWithAllowedOrigin() throws Exception { this.servletRequest.setServerName("mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET"); @@ -191,10 +192,22 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { this.service.setAllowedOrigins(Arrays.asList("*")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNotNull(this.service.getCorsConfiguration(this.servletRequest)); + } + @Test // SPR-16304 + public void handleInfoOptionsWithForbiddenOrigin() throws Exception { this.servletRequest.setServerName("mydomain3.com"); + this.servletRequest.addHeader(HttpHeaders.ORIGIN, "http://mydomain2.com"); + this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET"); + this.servletRequest.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, "Last-Modified"); + resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); + CorsConfiguration corsConfiguration = this.service.getCorsConfiguration(this.servletRequest); + assertTrue(corsConfiguration.getAllowedOrigins().isEmpty()); + this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); + corsConfiguration = this.service.getCorsConfiguration(this.servletRequest); + assertEquals(Arrays.asList("http://mydomain1.com"), corsConfiguration.getAllowedOrigins()); } @Test // SPR-12283