Skip to content

gh-3572 Upgrade to Spring Security 5.2 #3573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

gh-3572 Upgrade to Spring Security 5.2 #3573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b30440b
gh-3572 Upgrade to Spring Security 5.2
ghillert Oct 24, 2019
069fb2d
gh-3572 Code review changes
ghillert Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,24 @@

<!-- note - check version of reactor at deployer-cf/cf-java-client uses -->
<spring-cloud-deployer-cloudfoundry.version>2.1.0.BUILD-SNAPSHOT</spring-cloud-deployer-cloudfoundry.version>
<reactor.version>3.2.0.RELEASE</reactor.version>
<reactor.version>3.3.0.RELEASE</reactor.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that we're going to reactor 3.3.x when cf java client gets to it but why this change with this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a call in Spring Security WebClient that uses an API call of 3.3.x.

<spring-cloud-deployer-kubernetes.version>2.1.0.BUILD-SNAPSHOT</spring-cloud-deployer-kubernetes.version>
<kubernetes-client.version>4.1.0</kubernetes-client.version>

<spring-cloud-skipper.version>2.2.0.BUILD-SNAPSHOT</spring-cloud-skipper.version>

<spring-cloud-task.version>2.2.0.BUILD-SNAPSHOT</spring-cloud-task.version>

<spring-security-oauth2-autoconfigure.version>2.2.0.RC1</spring-security-oauth2-autoconfigure.version>
<!-- Only used for tests -->
<spring-security-oauth2.version>2.3.7.RELEASE</spring-security-oauth2.version>

<!-- Note: Make sure to update `spring-cloud-build` version and `spring-cloud-dependencies` version (spring-cloud.version)
are in sync with the https://github.com/spring-cloud/spring-cloud-release/blob/master/pom.xml updates for the respective
release versions -->
<spring-cloud.version>Hoxton.BUILD-SNAPSHOT</spring-cloud.version>
<java-cfenv-boot.version>1.1.1.RELEASE</java-cfenv-boot.version>

<spring-cloud-common-security-config.version>1.2.0.M2</spring-cloud-common-security-config.version>
<spring-cloud-common-security-config.version>1.2.0.BUILD-SNAPSHOT</spring-cloud-common-security-config.version>

<spring-shell.version>1.2.0.RELEASE</spring-shell.version>
<!-- Use older flyway until we can use flaway 6.x given to us from boot -->
Expand Down Expand Up @@ -224,13 +225,6 @@
<artifactId>commons-lang</artifactId>
<version>${commons-lang.version}</version>
</dependency>
<!-- Required to override the spring-cloud-sso-connector versions. Related to
https://github.com/spring-cloud/spring-cloud-skipper/issues/750 -->
<dependency>
<groupId>org.springframework.security.oauth.boot</groupId>
<artifactId>spring-security-oauth2-autoconfigure</artifactId>
<version>${spring-security-oauth2-autoconfigure.version}</version>
</dependency>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>kubernetes-client</artifactId>
Expand Down
333 changes: 212 additions & 121 deletions spring-cloud-dataflow-docs/src/main/asciidoc/configuration-local.adoc
View file
Open in desktop

Large diffs are not rendered by default.

47 changes: 19 additions & 28 deletions .../dataflow/server/config/cloudfoundry/security/CloudFoundryOAuthSecurityConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017 the original author or authors.
* Copyright 2017-2019 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.
Expand All @@ -24,20 +24,19 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnCloudPlatform;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.autoconfigure.security.oauth2.resource.PrincipalExtractor;
import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoTokenServices;
import org.springframework.boot.cloud.CloudPlatform;
import org.springframework.cloud.common.security.OAuthSecurityConfiguration;
import org.springframework.cloud.common.security.support.DefaultAuthoritiesExtractor;
import org.springframework.cloud.common.security.support.CustomAuthoritiesOpaqueTokenIntrospector;
import org.springframework.cloud.common.security.support.DefaultAuthoritiesMapper;
import org.springframework.cloud.common.security.support.OAuth2TokenUtilsService;
import org.springframework.cloud.common.security.support.OnOAuth2SecurityEnabled;
import org.springframework.cloud.dataflow.server.config.cloudfoundry.security.support.CloudFoundryDataflowAuthoritiesExtractor;
import org.springframework.cloud.dataflow.server.config.cloudfoundry.security.support.CloudFoundryPrincipalExtractor;
import org.springframework.cloud.dataflow.server.config.cloudfoundry.security.support.CloudFoundryDataflowAuthoritiesMapper;
import org.springframework.cloud.dataflow.server.config.cloudfoundry.security.support.CloudFoundrySecurityService;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.security.oauth2.client.OAuth2RestTemplate;
import org.springframework.web.client.RestTemplate;

/**
* When running inside Cloud Foundry, this {@link Configuration} class will reconfigure
Expand All @@ -48,7 +47,7 @@
* {@code Space Developers} have access to the underlying REST API's.
* <p>
* For this to happen, a REST call will be made to the Cloud Foundry Permissions API via
* CloudFoundrySecurityService inside the {@link DefaultAuthoritiesExtractor}.
* CloudFoundrySecurityService inside the {@link DefaultAuthoritiesMapper}.
* <p>
* If the user has the respective permissions, the CF_SPACE_DEVELOPER_ROLE will be
* assigned to the user.
Expand All @@ -68,26 +67,16 @@ public class CloudFoundryOAuthSecurityConfiguration {
private static final Logger logger = LoggerFactory.getLogger(CloudFoundryOAuthSecurityConfiguration.class);

@Autowired
private UserInfoTokenServices userInfoTokenServices;
private CustomAuthoritiesOpaqueTokenIntrospector customAuthoritiesOpaqueTokenIntrospector;

@Autowired(required = false)
private CloudFoundryDataflowAuthoritiesExtractor cloudFoundryDataflowAuthoritiesExtractor;

@Autowired(required = false)
private PrincipalExtractor principalExtractor;
private CloudFoundryDataflowAuthoritiesMapper cloudFoundryDataflowAuthoritiesExtractor;

@PostConstruct
public void init() {
if (this.cloudFoundryDataflowAuthoritiesExtractor != null) {
logger.info("Setting up Cloud Foundry AuthoritiesExtractor for UAA.");
this.userInfoTokenServices.setAuthoritiesExtractor(this.cloudFoundryDataflowAuthoritiesExtractor);
}
if (this.principalExtractor != null) {
logger.info("Setting up Cloud Foundry PrincipalExtractor.");
this.userInfoTokenServices.setPrincipalExtractor(this.principalExtractor);
}
else {
this.userInfoTokenServices.setPrincipalExtractor(new CloudFoundryPrincipalExtractor());
this.customAuthoritiesOpaqueTokenIntrospector.setAuthorityMapper(this.cloudFoundryDataflowAuthoritiesExtractor);
}
}

Expand All @@ -101,17 +90,19 @@ public class CloudFoundryUAAConfiguration {
@Value("${vcap.application.application_id}")
private String applicationId;

@Autowired
private OAuth2RestTemplate oAuth2RestTemplate;

@Bean
public CloudFoundryDataflowAuthoritiesExtractor authoritiesExtractor() {
return new CloudFoundryDataflowAuthoritiesExtractor(cloudFoundrySecurityService());
public CloudFoundryDataflowAuthoritiesMapper authoritiesExtractor(
CloudFoundrySecurityService cloudFoundrySecurityService
) {
return new CloudFoundryDataflowAuthoritiesMapper(cloudFoundrySecurityService);
}

@Bean
public CloudFoundrySecurityService cloudFoundrySecurityService() {
return new CloudFoundrySecurityService(this.oAuth2RestTemplate, this.cloudControllerUrl,
public CloudFoundrySecurityService cloudFoundrySecurityService(
OAuth2TokenUtilsService oauth2TokenUtilsService,
RestTemplate restTemplate) {
return new CloudFoundrySecurityService(oauth2TokenUtilsService, restTemplate,
this.cloudControllerUrl,
this.applicationId);
}

Expand Down
34 changes: 20 additions & 14 deletions ...dFoundryDataflowAuthoritiesExtractor.java → ...loudFoundryDataflowAuthoritiesMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017 the original author or authors.
* Copyright 2017-2019 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.
Expand All @@ -16,15 +16,16 @@
package org.springframework.cloud.dataflow.server.config.cloudfoundry.security.support;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.boot.autoconfigure.security.oauth2.resource.AuthoritiesExtractor;
import org.springframework.cloud.common.security.support.AuthoritiesMapper;
import org.springframework.cloud.common.security.support.CoreSecurityRoles;
import org.springframework.cloud.common.security.support.SecurityConfigUtils;
import org.springframework.security.config.core.GrantedAuthorityDefaults;
Expand All @@ -34,21 +35,21 @@
import org.springframework.util.StringUtils;

/**
* This Spring Cloud Data Flow {@link AuthoritiesExtractor} will assign all
* This Spring Cloud Data Flow {@link AuthoritiesMapper} will assign all
* {@link CoreSecurityRoles} to the authenticated OAuth2 user IF the user is a "Space
* Developer" in Cloud Foundry.
*
* @author Gunnar Hillert
*
*/
public class CloudFoundryDataflowAuthoritiesExtractor implements AuthoritiesExtractor {
public class CloudFoundryDataflowAuthoritiesMapper implements AuthoritiesMapper {

private static final Logger logger = LoggerFactory
.getLogger(CloudFoundryDataflowAuthoritiesExtractor.class);
.getLogger(CloudFoundryDataflowAuthoritiesMapper.class);

private final CloudFoundrySecurityService cloudFoundrySecurityService;

public CloudFoundryDataflowAuthoritiesExtractor(CloudFoundrySecurityService cloudFoundrySecurityService) {
public CloudFoundryDataflowAuthoritiesMapper(CloudFoundrySecurityService cloudFoundrySecurityService) {
this.cloudFoundrySecurityService = cloudFoundrySecurityService;
}

Expand All @@ -60,24 +61,29 @@ public CloudFoundryDataflowAuthoritiesExtractor(CloudFoundrySecurityService clou
* @param map Must not be null. Is only used for logging
*/
@Override
public List<GrantedAuthority> extractAuthorities(Map<String, Object> map) {
Assert.notNull(map, "The map argument must not be null.");
public Set<GrantedAuthority> mapScopesToAuthorities(Set<String> scopes) {
Assert.notNull(scopes, "The scopes argument must not be null.");

if (cloudFoundrySecurityService.isSpaceDeveloper()) {
final List<String> rolesAsStrings = new ArrayList<>();
final List<GrantedAuthority> grantedAuthorities = Stream.of(CoreSecurityRoles.values())
final Set<GrantedAuthority> grantedAuthorities = Stream.of(CoreSecurityRoles.values())
.map(roleEnum -> {
final String roleName = SecurityConfigUtils.ROLE_PREFIX + roleEnum.getKey();
rolesAsStrings.add(roleName);
return new SimpleGrantedAuthority(roleName);
})
.collect(Collectors.toList());
logger.info("Adding ALL roles {} to Cloud Foundry Space Developer user {}",
StringUtils.collectionToCommaDelimitedString(rolesAsStrings), map);
.collect(Collectors.toSet());
logger.info("Adding ALL roles {} to Cloud Foundry Space Developer user.",
StringUtils.collectionToCommaDelimitedString(rolesAsStrings));
return grantedAuthorities;
}
else {
return new ArrayList<>(0);
return Collections.emptySet();
}
}

@Override
public Set<GrantedAuthority> mapScopesToAuthorities(String providerId, Set<String> scopes) {
throw new UnsupportedOperationException("Don't call this AuthoritiesMapper with a providerId.");
}
}
43 changes: 0 additions & 43 deletions .../dataflow/server/config/cloudfoundry/security/support/CloudFoundryPrincipalExtractor.java
View file
Open in desktop

This file was deleted.

26 changes: 15 additions & 11 deletions ...oud/dataflow/server/config/cloudfoundry/security/support/CloudFoundrySecurityService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017 the original author or authors.
* Copyright 2017-2019 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.
Expand All @@ -22,13 +22,13 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.cloud.common.security.support.OAuth2TokenUtilsService;
import org.springframework.http.HttpStatus;
import org.springframework.http.RequestEntity;
import org.springframework.security.oauth2.client.OAuth2RestTemplate;
import org.springframework.security.oauth2.common.OAuth2AccessToken;
import org.springframework.util.Assert;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.HttpServerErrorException;
import org.springframework.web.client.RestTemplate;

/**
* Cloud Foundry security service to handle REST calls to the cloud controller and UAA.
Expand All @@ -42,20 +42,24 @@ public class CloudFoundrySecurityService {

private static final Logger logger = LoggerFactory.getLogger(CloudFoundrySecurityService.class);

private final OAuth2RestTemplate oAuth2RestTemplate;
private final OAuth2TokenUtilsService oauth2TokenUtilsService;
private final RestTemplate restTemplate;

private final String cloudControllerUrl;

private final String applicationId;

public CloudFoundrySecurityService(OAuth2RestTemplate oAuth2RestTemplate, String cloudControllerUrl,
public CloudFoundrySecurityService(OAuth2TokenUtilsService oauth2TokenUtilsService,
RestTemplate restTemplate, String cloudControllerUrl,
String applicationId) {
Assert.notNull(oAuth2RestTemplate, "OAuth2RestTemplate must not be null.");
Assert.notNull(oauth2TokenUtilsService, "oauth2TokenUtilsService must not be null.");
Assert.notNull(restTemplate, "restTemplate must not be null.");
Assert.notNull(cloudControllerUrl, "CloudControllerUrl must not be null.");
Assert.notNull(applicationId, "ApplicationId must not be null.");
this.oAuth2RestTemplate = oAuth2RestTemplate;
this.oauth2TokenUtilsService = oauth2TokenUtilsService;
this.cloudControllerUrl = cloudControllerUrl;
this.applicationId = applicationId;
this.restTemplate = restTemplate;
}

/**
Expand All @@ -66,10 +70,10 @@ public CloudFoundrySecurityService(OAuth2RestTemplate oAuth2RestTemplate, String
* @return true of the user is a space developer in Cloud Foundry
*/
public boolean isSpaceDeveloper() {
final OAuth2AccessToken accessToken = this.oAuth2RestTemplate.getAccessToken();
logger.info("The accessToken is: " + accessToken.getValue());
final String accessToken = this.oauth2TokenUtilsService.getAccessTokenOfAuthenticatedUser();
logger.info("The accessToken is: " + accessToken);
final AccessLevel accessLevel = getAccessLevel(
accessToken.getValue(), applicationId);
accessToken, applicationId);

if (AccessLevel.FULL.equals(accessLevel)) {
return true;
Expand All @@ -93,7 +97,7 @@ public AccessLevel getAccessLevel(String token, String applicationId)
logger.info("Using PermissionsUri: " + permissionsUri);
RequestEntity<?> request = RequestEntity.get(permissionsUri)
.header("Authorization", "bearer " + token).build();
Map<?, ?> body = this.oAuth2RestTemplate.exchange(request, Map.class).getBody();
Map<?, ?> body = this.restTemplate.exchange(request, Map.class).getBody();
if (Boolean.TRUE.equals(body.get("read_sensitive_data"))) {
return AccessLevel.FULL;
}
Expand Down
2 changes: 1 addition & 1 deletion .../java/org/springframework/cloud/dataflow/rest/client/config/DataFlowClientProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class DataFlowClientProperties {
/**
* Skip Ssl validation.
*/
private boolean skipSslValidation = true;
private boolean skipSslValidation;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we wanted to enable SSL validation by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should never "skip ssl validation" by default. I am not sure how that got set to true in the first place.



/**
Expand Down
6 changes: 4 additions & 2 deletions ...org/springframework/cloud/dataflow/server/config/DataFlowControllerAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.springframework.boot.web.client.RestTemplateBuilder;
import org.springframework.cloud.common.security.AuthorizationProperties;
import org.springframework.cloud.common.security.support.OAuth2AccessTokenProvidingClientHttpRequestInterceptor;
import org.springframework.cloud.common.security.support.OAuth2TokenUtilsService;
import org.springframework.cloud.common.security.support.SecurityStateBean;
import org.springframework.cloud.dataflow.audit.repository.AuditRecordRepository;
import org.springframework.cloud.dataflow.audit.service.AuditRecordService;
Expand Down Expand Up @@ -363,7 +364,8 @@ public StreamDeploymentController updatableStreamDeploymentController(

@Bean
public SkipperClient skipperClient(SkipperClientProperties properties,
RestTemplateBuilder restTemplateBuilder, ObjectMapper objectMapper) {
RestTemplateBuilder restTemplateBuilder, ObjectMapper objectMapper,
@Nullable OAuth2TokenUtilsService oauth2TokenUtilsService) {

// TODO (Tzolov) review the manual Hal convertion configuration
objectMapper.registerModule(new Jackson2HalModule());
Expand All @@ -373,7 +375,7 @@ public SkipperClient skipperClient(SkipperClientProperties properties,

RestTemplate restTemplate = restTemplateBuilder
.errorHandler(new SkipperClientResponseErrorHandler(objectMapper))
.interceptors(new OAuth2AccessTokenProvidingClientHttpRequestInterceptor())
.interceptors(new OAuth2AccessTokenProvidingClientHttpRequestInterceptor(oauth2TokenUtilsService))
.messageConverters(Arrays.asList(new StringHttpMessageConverter(),
new MappingJackson2HttpMessageConverter(objectMapper)))
.build();
Expand Down
Loading