From 43d3abdfd54697d90d4993f5d1975c6f384455a2 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 79e773be697..ab04e9eccfd 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-2017 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.StandardCharsets; +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 f759bb49b3a..b9af506f2bf 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. @@ -32,6 +32,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; @@ -176,7 +177,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"); @@ -196,10 +197,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