Merge branch 'master' of https://github.com/spring-projects/spring-security into spring-projectsgh-7791

Author: sdratler1

.github/workflows/pr-build-workflow.yml

@@ -0,0 +1,25 @@
+name: PR Build
+
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up JDK
+        uses: actions/setup-java@v1
+        with:
+          java-version: '8'
+      - name: Cache Gradle packages
+        uses: actions/cache@v2
+        with:
+          path: ~/.gradle/caches
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+      - name: Build with Gradle
+        run: ./gradlew clean build --continue

CONTRIBUTING.adoc

@@ -140,7 +140,7 @@ git config user.email link:mailto:user@ma&#x69
 . Keep the subject line to 50 characters or less if possible
 . Do not end the subject line with a period
 . In the body of the commit message, explain how things worked before this commit, what has changed, and how things work now
-. Include Fixes gh-<issue-number> at the end if this fixes a GitHub issue
+. Include Closes gh-<issue-number> at the end if this fixes a GitHub issue
 . Avoid markdown, including back-ticks identifying code
 
 = Run all tests prior to submission

build.gradle

@@ -2,7 +2,7 @@ buildscript {
 	dependencies {
 		classpath 'io.spring.gradle:spring-build-conventions:0.0.32.RELEASE'
 		classpath "org.springframework.boot:spring-boot-gradle-plugin:$springBootVersion"
-		classpath 'io.spring.nohttp:nohttp-gradle:0.0.2.RELEASE'
+		classpath 'io.spring.nohttp:nohttp-gradle:0.0.5.RELEASE'
 		classpath "io.freefair.gradle:aspectj-plugin:5.0.1"
 		classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion"
 	}
@@ -39,3 +39,7 @@ subprojects {
 		options.encoding = "UTF-8"
 	}
 }
+
+nohttp {
+	allowlistFile = project.file("etc/nohttp/allowlist.lines")
+}

config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.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.
@@ -31,6 +31,7 @@
 import org.springframework.http.HttpMethod;
 import org.springframework.security.access.PermissionEvaluator;
 import org.springframework.security.access.expression.SecurityExpressionHandler;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
 import org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder;
 import org.springframework.security.config.annotation.ObjectPostProcessor;
 import org.springframework.security.config.annotation.SecurityBuilder;
@@ -75,6 +76,7 @@
  * @see WebSecurityConfiguration
  *
  * @author Rob Winch
+ * @author Evgeniy Cheban
  * @since 3.2
  */
 public final class WebSecurity extends
@@ -389,6 +391,11 @@ public void setApplicationContext(ApplicationContext applicationContext)
 			throws BeansException {
 		this.defaultWebSecurityExpressionHandler
 				.setApplicationContext(applicationContext);
+
+		try {
+			this.defaultWebSecurityExpressionHandler.setRoleHierarchy(applicationContext.getBean(RoleHierarchy.class));
+		} catch (NoSuchBeanDefinitionException e) {}
+
 		try {
 			this.defaultWebSecurityExpressionHandler.setPermissionEvaluator(applicationContext.getBean(
 					PermissionEvaluator.class));

config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java

@@ -187,7 +187,7 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception {
 	/**
 	 * Creates the {@link HttpSecurity} or returns the current instance
 	 *
-	 * ] * @return the {@link HttpSecurity}
+	 * @return the {@link HttpSecurity}
 	 * @throws Exception
 	 */
 	@SuppressWarnings({ "rawtypes", "unchecked" })

config/src/main/java/org/springframework/security/config/annotation/web/reactive/ServerHttpSecurityConfiguration.java

@@ -18,6 +18,7 @@
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -87,11 +88,11 @@ void setUserDetailsPasswordService(ReactiveUserDetailsPasswordService userDetail
 
 	@Bean
 	public WebFluxConfigurer authenticationPrincipalArgumentResolverConfigurer(
-			AuthenticationPrincipalArgumentResolver authenticationPrincipalArgumentResolver) {
+			ObjectProvider<AuthenticationPrincipalArgumentResolver> authenticationPrincipalArgumentResolver) {
 		return new WebFluxConfigurer() {
 			@Override
 			public void configureArgumentResolvers(ArgumentResolverConfigurer configurer) {
-				configurer.addCustomResolver(authenticationPrincipalArgumentResolver);
+				configurer.addCustomResolver(authenticationPrincipalArgumentResolver.getObject());
 			}
 		};
 	}

config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java

@@ -32,6 +32,8 @@
 import java.util.function.Supplier;
 
 import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
+import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
+import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
 import reactor.core.publisher.Mono;
 import reactor.util.context.Context;
 
@@ -236,6 +238,7 @@
  * @author Rafiullah Hamedy
  * @author Eddú Meléndez
  * @author Joe Grandja
+ * @author Parikshit Dutta
  * @since 5.0
  */
 public class ServerHttpSecurity {
@@ -1093,9 +1096,14 @@ public OAuth2LoginSpec authenticationConverter(ServerAuthenticationConverter aut
 
 		private ServerAuthenticationConverter getAuthenticationConverter(ReactiveClientRegistrationRepository clientRegistrationRepository) {
 			if (this.authenticationConverter == null) {
-				ServerOAuth2AuthorizationCodeAuthenticationTokenConverter authenticationConverter = new ServerOAuth2AuthorizationCodeAuthenticationTokenConverter(clientRegistrationRepository);
-				authenticationConverter.setAuthorizationRequestRepository(getAuthorizationRequestRepository());
+				ServerOAuth2AuthorizationCodeAuthenticationTokenConverter delegate =
+						new ServerOAuth2AuthorizationCodeAuthenticationTokenConverter(clientRegistrationRepository);
+				delegate.setAuthorizationRequestRepository(getAuthorizationRequestRepository());
+				ServerAuthenticationConverter authenticationConverter = exchange ->
+						delegate.convert(exchange).onErrorMap(OAuth2AuthorizationException.class,
+								e -> new OAuth2AuthenticationException(e.getError(), e.getError().toString()));
 				this.authenticationConverter = authenticationConverter;
+				return authenticationConverter;
 			}
 			return this.authenticationConverter;
 		}
@@ -1511,10 +1519,17 @@ protected void configure(ServerHttpSecurity http) {
 			OAuth2AuthorizationCodeGrantWebFilter codeGrantWebFilter = new OAuth2AuthorizationCodeGrantWebFilter(
 					authenticationManager, authenticationConverter, authorizedClientRepository);
 			codeGrantWebFilter.setAuthorizationRequestRepository(getAuthorizationRequestRepository());
+			if (http.requestCache != null) {
+				codeGrantWebFilter.setRequestCache(http.requestCache.requestCache);
+			}
 
 			OAuth2AuthorizationRequestRedirectWebFilter oauthRedirectFilter = new OAuth2AuthorizationRequestRedirectWebFilter(
 					clientRegistrationRepository);
 			oauthRedirectFilter.setAuthorizationRequestRepository(getAuthorizationRequestRepository());
+			if (http.requestCache != null) {
+				oauthRedirectFilter.setRequestCache(http.requestCache.requestCache);
+			}
+
 			http.addFilterAt(codeGrantWebFilter, SecurityWebFiltersOrder.OAUTH2_AUTHORIZATION_CODE);
 			http.addFilterAt(oauthRedirectFilter, SecurityWebFiltersOrder.HTTP_BASIC);
 		}

config/src/main/resources/org/springframework/security/config/spring-security-5.4.rnc

@@ -1034,6 +1034,10 @@ frame-options.attlist &=
 	attribute value {xsd:string}?
 frame-options.attlist &=
 	## Specify the request parameter to use for the origin when using a 'whitelist' or 'regexp' based strategy. Default is 'from'.
+	## Deprecated ALLOW-FROM is an obsolete directive that no longer works in modern browsers. Instead use
+	## Content-Security-Policy with the
+	## <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors">frame-ancestors</a>
+	## directive.
 	attribute from-parameter {xsd:string}?
 
 

config/src/main/resources/org/springframework/security/config/spring-security-5.4.xsd

@@ -3000,7 +3000,10 @@
       <xs:attribute name="from-parameter" type="xs:string">
          <xs:annotation>
             <xs:documentation>Specify the request parameter to use for the origin when using a 'whitelist' or 'regexp'
-                based strategy. Default is 'from'.
+                based strategy. Default is 'from'. Deprecated ALLOW-FROM is an obsolete directive that no
+                longer works in modern browsers. Instead use Content-Security-Policy with the &lt;a
+                href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors"&gt;frame-ancestors&lt;/a&gt;
+                directive.
                 </xs:documentation>
          </xs:annotation>
       </xs:attribute>

config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.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.
@@ -32,6 +32,8 @@
 import org.springframework.security.access.PermissionEvaluator;
 import org.springframework.security.access.expression.AbstractSecurityExpressionHandler;
 import org.springframework.security.access.expression.SecurityExpressionHandler;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
 import org.springframework.security.config.annotation.authentication.configuration.EnableGlobalAuthentication;
@@ -69,6 +71,7 @@
  *
  * @author Rob Winch
  * @author Joe Grandja
+ * @author Evgeniy Cheban
  */
 public class WebSecurityConfigurationTests {
 	@Rule
@@ -290,6 +293,31 @@ protected void configure(HttpSecurity http) throws Exception {
 		}
 	}
 
+	@Test
+	public void securityExpressionHandlerWhenRoleHierarchyBeanThenRoleHierarchyUsed() {
+		this.spring.register(WebSecurityExpressionHandlerRoleHierarchyBeanConfig.class).autowire();
+		TestingAuthenticationToken authentication = new TestingAuthenticationToken("user", "notused", "ROLE_ADMIN");
+		FilterInvocation invocation = new FilterInvocation(new MockHttpServletRequest("GET", ""),
+				new MockHttpServletResponse(), new MockFilterChain());
+
+		AbstractSecurityExpressionHandler handler = this.spring.getContext().getBean(AbstractSecurityExpressionHandler.class);
+		EvaluationContext evaluationContext = handler.createEvaluationContext(authentication, invocation);
+		Expression expression = handler.getExpressionParser()
+				.parseExpression("hasRole('ROLE_USER')");
+		boolean granted = expression.getValue(evaluationContext, Boolean.class);
+		assertThat(granted).isTrue();
+	}
+
+	@EnableWebSecurity
+	static class WebSecurityExpressionHandlerRoleHierarchyBeanConfig extends WebSecurityConfigurerAdapter {
+		@Bean
+		RoleHierarchy roleHierarchy() {
+			RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl();
+			roleHierarchy.setHierarchy("ROLE_ADMIN > ROLE_USER");
+			return roleHierarchy;
+		}
+	}
+
 	@Test
 	public void securityExpressionHandlerWhenPermissionEvaluatorBeanThenPermissionEvaluatorUsed() {
 		this.spring.register(WebSecurityExpressionHandlerPermissionEvaluatorBeanConfig.class).autowire();

config/src/test/java/org/springframework/security/config/annotation/web/reactive/EnableWebFluxSecurityTests.java

@@ -47,6 +47,7 @@
 import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners;
 import org.springframework.security.test.context.support.WithMockUser;
 import org.springframework.security.test.web.reactive.server.WebTestClientBuilder;
+import org.springframework.security.web.reactive.result.method.annotation.AuthenticationPrincipalArgumentResolver;
 import org.springframework.security.web.reactive.result.view.CsrfRequestDataValueProcessor;
 import org.springframework.security.web.server.SecurityWebFilterChain;
 import org.springframework.security.web.server.WebFilterChainProxy;
@@ -59,6 +60,7 @@
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.reactive.config.DelegatingWebFluxConfiguration;
 import org.springframework.web.reactive.config.EnableWebFlux;
 import org.springframework.web.reactive.function.BodyInserters;
 import org.springframework.web.reactive.result.view.AbstractView;
@@ -434,4 +436,23 @@ static class Child {
 		Child() {
 		}
 	}
+
+	@Test
+	// gh-8596
+	public void resolveAuthenticationPrincipalArgumentResolverFirstDoesNotCauseBeanCurrentlyInCreationException() {
+		this.spring.register(EnableWebFluxSecurityConfiguration.class,
+				ReactiveAuthenticationTestConfiguration.class,
+				DelegatingWebFluxConfiguration.class).autowire();
+	}
+
+	@EnableWebFluxSecurity
+	@Configuration(proxyBeanMethods = false)
+	static class EnableWebFluxSecurityConfiguration {
+		/**
+		 * It is necessary to Autowire AuthenticationPrincipalArgumentResolver because it triggers eager loading of
+		 * AuthenticationPrincipalArgumentResolver bean which causes BeanCurrentlyInCreationException
+		 */
+		@Autowired
+		AuthenticationPrincipalArgumentResolver resolver;
+	}
 }

config/src/test/java/org/springframework/security/config/doc/XsdDocumentedTests.java

@@ -52,7 +52,12 @@ public class XsdDocumentedTests {
 		"nsa-websocket-security",
 		"nsa-ldap",
 		"nsa-method-security",
-		"nsa-web");
+		"nsa-web",
+		// deprecated and for removal
+		"nsa-frame-options-strategy",
+		"nsa-frame-options-ref",
+		"nsa-frame-options-value",
+		"nsa-frame-options-from-parameter");
 
 	String referenceLocation = "../docs/manual/src/docs/asciidoc/_includes/servlet/appendix/namespace.adoc";