From 70773468c2ac06e5f5680bd0808f52305554fa33 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 20 Jul 2020 06:32:38 +0300 Subject: [PATCH] Align default values with 5.0.x Closes gh-25414 --- .../web/bind/annotation/CrossOrigin.java | 9 ++++--- .../web/cors/CorsConfiguration.java | 5 +--- .../config/annotation/CorsRegistration.java | 9 ++++--- .../web/servlet/config/MvcNamespaceTests.java | 10 ++++---- .../method/annotation/CrossOriginTests.java | 25 +++++++++++-------- .../web/servlet/config/mvc-config-cors.xml | 2 +- src/asciidoc/web-cors.adoc | 12 +++++++-- 7 files changed, 42 insertions(+), 30 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java index 75264d48e974..5da206141b68 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 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. @@ -28,7 +28,7 @@ /** * Marks the annotated method or type as permitting cross origin requests. * - *

By default all origins and headers are permitted, credentials are allowed, + *

By default all origins and headers are permitted, credentials are not allowed, * and the maximum age is set to 1800 seconds (30 minutes). The list of HTTP * methods is set to the methods on the {@code @RequestMapping} if not * explicitly set on {@code @CrossOrigin}. @@ -67,7 +67,7 @@ * @deprecated as of Spring 4.3.4, in favor of using {@link CorsConfiguration#applyPermitDefaultValues} */ @Deprecated - boolean DEFAULT_ALLOW_CREDENTIALS = true; + boolean DEFAULT_ALLOW_CREDENTIALS = false; /** * @deprecated as of Spring 4.3.4, in favor of using {@link CorsConfiguration#applyPermitDefaultValues} @@ -133,7 +133,8 @@ * An empty string ({@code ""}) means undefined. * {@code "true"} means that the pre-flight response will include the header * {@code Access-Control-Allow-Credentials=true}. - *

If undefined, credentials are allowed. + *

If undefined, this is set to {@code "false"} in which case credentials + * are not allowed. */ String allowCredentials() default ""; diff --git a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java index 32e367737213..645c262f8ae5 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java +++ b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -328,9 +328,6 @@ public CorsConfiguration applyPermitDefaultValues() { if (this.allowedHeaders == null) { this.addAllowedHeader(ALL); } - if (this.allowCredentials == null) { - this.setAllowCredentials(true); - } if (this.maxAge == null) { this.setMaxAge(1800L); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/CorsRegistration.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/CorsRegistration.java index 92b26af0e9d6..de648d66df65 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/CorsRegistration.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/CorsRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 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. @@ -122,9 +122,10 @@ public CorsRegistration maxAge(long maxAge) { } /** - * Whether user credentials are supported. - *

By default this is set to {@code true} in which case user credentials - * are supported. + * Whether user credentials are supported in which case the browser should + * include any cookies associated with the domain of the request being + * annotated. + *

By default this is {@code false} and user credentials are not allowed. */ public CorsRegistration allowCredentials(boolean allowCredentials) { this.config.setAllowCredentials(allowCredentials); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java index 4b0ab3f60d52..a50d9e445414 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/MvcNamespaceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -920,13 +920,13 @@ public void testCorsMinimal() throws Exception { assertArrayEquals(new String[]{"GET", "HEAD", "POST"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[]{"*"}, config.getAllowedHeaders().toArray()); assertNull(config.getExposedHeaders()); - assertTrue(config.getAllowCredentials()); + assertNull(config.getAllowCredentials()); assertEquals(new Long(1800), config.getMaxAge()); } } @Test - public void testCors() throws Exception { + public void testCors() { loadBeanDefinitions("mvc-config-cors.xml"); String[] beanNames = appContext.getBeanNamesForType(AbstractHandlerMapping.class); @@ -943,14 +943,14 @@ public void testCors() throws Exception { assertArrayEquals(new String[]{"GET", "PUT"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[]{"header1", "header2", "header3"}, config.getAllowedHeaders().toArray()); assertArrayEquals(new String[]{"header1", "header2"}, config.getExposedHeaders().toArray()); - assertFalse(config.getAllowCredentials()); + assertTrue(config.getAllowCredentials()); assertEquals(Long.valueOf(123), config.getMaxAge()); config = configs.get("/resources/**"); assertArrayEquals(new String[]{"https://domain1.com"}, config.getAllowedOrigins().toArray()); assertArrayEquals(new String[]{"GET", "HEAD", "POST"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[]{"*"}, config.getAllowedHeaders().toArray()); assertNull(config.getExposedHeaders()); - assertTrue(config.getAllowCredentials()); + assertNull(config.getAllowCredentials()); assertEquals(Long.valueOf(1800), config.getMaxAge()); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java index cf09f18815be..26c45e0338ae 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2020 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. @@ -21,7 +21,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; -import java.util.Arrays; +import java.util.Collections; import java.util.Properties; import org.junit.Before; @@ -53,8 +53,13 @@ import org.springframework.web.servlet.mvc.condition.RequestMethodsRequestCondition; import org.springframework.web.servlet.mvc.method.RequestMappingInfo; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Test fixture for {@link CrossOrigin @CrossOrigin} annotated methods. @@ -123,7 +128,7 @@ public void defaultAnnotation() throws Exception { assertNotNull(config); assertArrayEquals(new String[] {"GET"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[] {"*"}, config.getAllowedOrigins().toArray()); - assertTrue(config.getAllowCredentials()); + assertNull(config.getAllowCredentials()); assertArrayEquals(new String[] {"*"}, config.getAllowedHeaders().toArray()); assertTrue(CollectionUtils.isEmpty(config.getExposedHeaders())); assertEquals(new Long(1800), config.getMaxAge()); @@ -151,8 +156,8 @@ public void customOriginDefinedViaValueAttribute() throws Exception { HandlerExecutionChain chain = this.handlerMapping.getHandler(request); CorsConfiguration config = getCorsConfiguration(chain, false); assertNotNull(config); - assertEquals(Arrays.asList("https://example.com"), config.getAllowedOrigins()); - assertTrue(config.getAllowCredentials()); + assertEquals(Collections.singletonList("https://example.com"), config.getAllowedOrigins()); + assertNull(config.getAllowCredentials()); } @Test @@ -162,8 +167,8 @@ public void customOriginDefinedViaPlaceholder() throws Exception { HandlerExecutionChain chain = this.handlerMapping.getHandler(request); CorsConfiguration config = getCorsConfiguration(chain, false); assertNotNull(config); - assertEquals(Arrays.asList("https://example.com"), config.getAllowedOrigins()); - assertTrue(config.getAllowCredentials()); + assertEquals(Collections.singletonList("https://example.com"), config.getAllowedOrigins()); + assertNull(config.getAllowCredentials()); } @Test @@ -240,7 +245,7 @@ public void preFlightRequest() throws Exception { assertNotNull(config); assertArrayEquals(new String[] {"GET"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[] {"*"}, config.getAllowedOrigins().toArray()); - assertTrue(config.getAllowCredentials()); + assertNull(config.getAllowCredentials()); assertArrayEquals(new String[] {"*"}, config.getAllowedHeaders().toArray()); assertTrue(CollectionUtils.isEmpty(config.getExposedHeaders())); assertEquals(new Long(1800), config.getMaxAge()); diff --git a/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-cors.xml b/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-cors.xml index bc4e2d77fe0c..32a199d3556e 100644 --- a/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-cors.xml +++ b/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-cors.xml @@ -11,7 +11,7 @@ + exposed-headers="header1, header2" allow-credentials="true" max-age="123" /> diff --git a/src/asciidoc/web-cors.adoc b/src/asciidoc/web-cors.adoc index 80678bc1b716..6f389b74a4de 100644 --- a/src/asciidoc/web-cors.adoc +++ b/src/asciidoc/web-cors.adoc @@ -24,6 +24,13 @@ implementation (https://github.com/spring-projects/spring-framework/blob/master/ by default) in order to add the relevant CORS response headers (like `Access-Control-Allow-Origin`) based on the CORS configuration you have provided. +[NOTE] +==== +Be aware that cookies are not allowed by default to avoid increasing the surface attack of +the web application (for example via exposing sensitive user-specific information like +CSRF tokens). Set `allowedCredentials` property to `true` in order to allow them. +==== + [NOTE] ==== Since CORS requests are automatically dispatched, you *do not need* to change the @@ -151,7 +158,8 @@ public class WebConfig extends WebMvcConfigurerAdapter { .allowedMethods("PUT", "DELETE") .allowedHeaders("header1", "header2", "header3") .exposedHeaders("header1", "header2") - .allowCredentials(false).maxAge(3600); + .allowCredentials(true) + .maxAge(3600); } } ---- @@ -180,7 +188,7 @@ It is also possible to declare several CORS mappings with customized properties: allowed-origins="https://domain1.com, https://domain2.com" allowed-methods="GET, PUT" allowed-headers="header1, header2, header3" - exposed-headers="header1, header2" allow-credentials="false" + exposed-headers="header1, header2" max-age="123" />