Align default values with 5.0.x

rstoyanchev · rstoyanchev · commit 70773468c2ac · 2020-07-20T06:32:38.000+03:00
Closes gh-25414
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
@@ -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.
  *
- * <p>By default all origins and headers are permitted, credentials are allowed,
+ * <p>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 <em>undefined</em>.
 	 * {@code "true"} means that the pre-flight response will include the header
 	 * {@code Access-Control-Allow-Credentials=true}.
-	 * <p>If undefined, credentials are allowed.
+	 * <p>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
@@ -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
@@ -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.
-	 * <p>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.
+	 * <p>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
@@ -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
@@ -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
@@ -11,7 +11,7 @@
 
 		<mvc:mapping path="/api/**" allowed-origins="https://domain1.com, https://domain2.com"
 					 allowed-methods="GET, PUT" allowed-headers="header1, header2, header3"
-					 exposed-headers="header1, header2" allow-credentials="false" max-age="123" />
+					 exposed-headers="header1, header2" allow-credentials="true" max-age="123" />
 
 		<mvc:mapping path="/resources/**" allowed-origins="https://domain1.com" />
 
diff --git 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" />
 
 	<mvc:mapping path="/resources/**"

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2019 the original author or authors.`
	`2`	`+ * Copyright 2002-2020 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -328,9 +328,6 @@ public CorsConfiguration applyPermitDefaultValues() {`
`328`	`328`	`if (this.allowedHeaders == null) {`
`329`	`329`	`this.addAllowedHeader(ALL);`
`330`	`330`	`}`
`331`		`- if (this.allowCredentials == null) {`
`332`		`- this.setAllowCredentials(true);`
`333`		`- }`
`334`	`331`	`if (this.maxAge == null) {`
`335`	`332`	`this.setMaxAge(1800L);`
`336`	`333`	`}`