Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,17 @@ static class OAuth2ClientWebMvcSecurityConfiguration implements WebMvcConfigurer
@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers) {
if (this.clientRegistrationRepository != null && this.authorizedClientRepository != null) {
OAuth2AuthorizedClientProvider authorizedClientProvider =
OAuth2AuthorizedClientProviderBuilder authorizedClientProviderBuilder =
OAuth2AuthorizedClientProviderBuilder.builder()
.authorizationCode()
.refreshToken()
.clientCredentials(configurer ->
Optional.ofNullable(this.accessTokenResponseClient).ifPresent(configurer::accessTokenResponseClient))
.build();
.refreshToken();

if (this.accessTokenResponseClient != null) {
authorizedClientProviderBuilder.clientCredentials(configurer ->
configurer.accessTokenResponseClient(this.accessTokenResponseClient));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This code changes the existing functionality. If accessTokenResponseClient is null, clientCredentials should still be enabled. Please add an else statement containing authorizedClientProviderBuilder.clientCredentials() to ensure clientCredentials is enabled 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.

Updated in 3fef167

OAuth2AuthorizedClientProvider authorizedClientProvider = authorizedClientProviderBuilder.build();

DefaultOAuth2AuthorizedClientManager authorizedClientManager = new DefaultOAuth2AuthorizedClientManager(
this.clientRegistrationRepository, this.authorizedClientRepository);
authorizedClientManager.setAuthorizedClientProvider(authorizedClientProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.function.Function;
import java.util.function.Supplier;
Expand Down Expand Up @@ -2665,10 +2664,10 @@ public ServerHttpSecurity disable() {
}

protected void configure(ServerHttpSecurity http) {
Optional.ofNullable(this.csrfTokenRepository).ifPresent(serverCsrfTokenRepository -> {
this.filter.setCsrfTokenRepository(serverCsrfTokenRepository);
http.logout().addLogoutHandler(new CsrfServerLogoutHandler(serverCsrfTokenRepository));
});
if (this.csrfTokenRepository != null) {
this.filter.setCsrfTokenRepository(this.csrfTokenRepository);
http.logout().addLogoutHandler(new CsrfServerLogoutHandler(this.csrfTokenRepository));
}
http.addFilterAt(this.filter, SecurityWebFiltersOrder.CSRF);
}

Expand Down Expand Up @@ -3607,19 +3606,21 @@ public ServerHttpSecurity disable() {
return and();
}

private Optional<ServerLogoutHandler> createLogoutHandler() {
private ServerLogoutHandler createLogoutHandler() {
if (this.logoutHandlers.isEmpty()) {
return Optional.empty();
}
else if (this.logoutHandlers.size() == 1) {
return Optional.of(this.logoutHandlers.get(0));
return null;
} else if (this.logoutHandlers.size() == 1) {
return this.logoutHandlers.get(0);
} else {
return new DelegatingServerLogoutHandler(this.logoutHandlers);
}

return Optional.of(new DelegatingServerLogoutHandler(this.logoutHandlers));
}

protected void configure(ServerHttpSecurity http) {
createLogoutHandler().ifPresent(this.logoutWebFilter::setLogoutHandler);
ServerLogoutHandler logoutHandler = createLogoutHandler();
if (logoutHandler != null) {
this.logoutWebFilter.setLogoutHandler(logoutHandler);
}
http.addFilterAt(this.logoutWebFilter, SecurityWebFiltersOrder.LOGOUT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Optional;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

Expand Down Expand Up @@ -52,25 +51,32 @@ public OidcClientInitiatedLogoutSuccessHandler(ClientRegistrationRepository clie
@Override
protected String determineTargetUrl(HttpServletRequest request,
HttpServletResponse response, Authentication authentication) {
String targetUrl = null;
URI endSessionEndpoint;
if (authentication instanceof OAuth2AuthenticationToken && authentication.getPrincipal() instanceof OidcUser) {
endSessionEndpoint = this.endSessionEndpoint((OAuth2AuthenticationToken) authentication);
if (endSessionEndpoint != null) {
targetUrl = endpointUri(endSessionEndpoint, authentication);
}
}
if (targetUrl == null) {
targetUrl = super.determineTargetUrl(request, response);
}

return Optional.of(authentication)
.filter(OAuth2AuthenticationToken.class::isInstance)
.filter(token -> authentication.getPrincipal() instanceof OidcUser)
.map(OAuth2AuthenticationToken.class::cast)
.flatMap(this::endSessionEndpoint)
.map(endSessionEndpoint -> endpointUri(endSessionEndpoint, authentication))
.orElseGet(() -> super.determineTargetUrl(request, response));
return targetUrl;
}

private Optional<URI> endSessionEndpoint(OAuth2AuthenticationToken token) {
private URI endSessionEndpoint(OAuth2AuthenticationToken token) {
String registrationId = token.getAuthorizedClientRegistrationId();
return Optional.of(
this.clientRegistrationRepository.findByRegistrationId(registrationId))
.map(ClientRegistration::getProviderDetails)
.map(ClientRegistration.ProviderDetails::getConfigurationMetadata)
.map(configurationMetadata -> configurationMetadata.get("end_session_endpoint"))
.map(Object::toString)
.map(URI::create);
ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(registrationId);
Copy link
Contributor

Choose a reason for hiding this comment

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

clientRegistration may be null. Please add a null check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6a4425c

Object endSessionEndpoint = clientRegistration.getProviderDetails().getConfigurationMetadata().get("end_session_endpoint");

URI result = null;
if (endSessionEndpoint != null) {
result = URI.create(endSessionEndpoint.toString());
}

return result;
}

private String endpointUri(URI endSessionEndpoint, Authentication authentication) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import com.nimbusds.oauth2.sdk.GrantType;
import com.nimbusds.oauth2.sdk.ParseException;
Expand Down Expand Up @@ -141,8 +140,11 @@ public static ClientRegistration.Builder fromIssuerLocation(String issuer) {
Map<String, Object> configuration = getConfiguration(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer);
return Optional.ofNullable((String) configuration.get("userinfo_endpoint"))
.map(builder::userInfoUri).orElse(builder);
String userinfoEndpoint = (String) configuration.get("userinfo_endpoint");
if (userinfoEndpoint != null) {
builder.userInfoUri(userinfoEndpoint);
}
return builder;
}

private static URI oidc(URI issuer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
package org.springframework.security.oauth2.server.resource.authentication;

import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import org.springframework.http.HttpStatus;
import org.springframework.security.authentication.AbstractAuthenticationToken;
Expand Down Expand Up @@ -128,11 +128,12 @@ private AbstractAuthenticationToken convert(String token, Map<String, Object> cl
}

private Collection<GrantedAuthority> extractAuthorities(Map<String, Object> claims) {
Collection<String> scopes = (Collection<String>) claims.get(SCOPE);
return Optional.ofNullable(scopes).orElse(Collections.emptyList())
.stream()
.map(authority -> new SimpleGrantedAuthority("SCOPE_" + authority))
.collect(Collectors.toList());
Collection<String> scopes = (Collection<String>) claims.getOrDefault(SCOPE, Collections.emptyList());
List<GrantedAuthority> authorities = new ArrayList<>();
for (String scope : scopes) {
authorities.add(new SimpleGrantedAuthority("SCOPE_" + scope));
}
return authorities;
}

private static BearerTokenError invalidToken(String message) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
package org.springframework.security.oauth2.server.resource.authentication;

import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import org.springframework.security.oauth2.core.OAuth2TokenAttributes;
import reactor.core.publisher.Mono;
Expand Down Expand Up @@ -108,11 +108,12 @@ private Mono<OAuth2IntrospectionAuthenticationToken> authenticate(String token)
}

private Collection<GrantedAuthority> extractAuthorities(Map<String, Object> claims) {
Collection<String> scopes = (Collection<String>) claims.get(SCOPE);
return Optional.ofNullable(scopes).orElse(Collections.emptyList())
.stream()
.map(authority -> new SimpleGrantedAuthority("SCOPE_" + authority))
.collect(Collectors.toList());
Collection<String> scopes = (Collection<String>) claims.getOrDefault(SCOPE, Collections.emptyList());
List<GrantedAuthority> authorities = new ArrayList<>();
for (String scope : scopes) {
authorities.add(new SimpleGrantedAuthority("SCOPE_" + scope));
}
return authorities;
}

private static BearerTokenError invalidToken(String message) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import com.nimbusds.oauth2.sdk.TokenIntrospectionResponse;
Expand Down Expand Up @@ -124,16 +123,22 @@ private MultiValueMap<String, String> requestBody(String token) {
*/
@Override
public Map<String, Object> introspect(String token) {
TokenIntrospectionSuccessResponse response = Optional.of(token)
.map(this.requestEntityConverter::convert)
.map(this::makeRequest)
.map(this::adaptToNimbusResponse)
.map(this::parseNimbusResponse)
.map(this::castToNimbusSuccess)
// relying solely on the authorization server to validate this token (not checking 'exp', for example)
.filter(TokenIntrospectionSuccessResponse::isActive)
.orElseThrow(() -> new OAuth2IntrospectionException("Provided token [" + token + "] isn't active"));
return convertClaimsSet(response);
RequestEntity<?> requestEntity = this.requestEntityConverter.convert(token);
if (requestEntity == null) {
throw new OAuth2IntrospectionException("Provided token [" + token + "] isn't active");
}

ResponseEntity<String> responseEntity = makeRequest(requestEntity);
HTTPResponse httpResponse = adaptToNimbusResponse(responseEntity);
TokenIntrospectionResponse introspectionResponse = parseNimbusResponse(httpResponse);
TokenIntrospectionSuccessResponse introspectionSuccessResponse = castToNimbusSuccess(introspectionResponse);

// relying solely on the authorization server to validate this token (not checking 'exp', for example)
if (!introspectionSuccessResponse.isActive()) {
throw new OAuth2IntrospectionException("Provided token [" + token + "] isn't active");
}

return convertClaimsSet(introspectionSuccessResponse);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to change this file since it is a test. Please revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted in f6f4378

import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -331,10 +330,14 @@ private void assertIndexPage(HtmlPage page) throws Exception {
}

private HtmlAnchor getClientAnchorElement(HtmlPage page, ClientRegistration clientRegistration) {
Optional<HtmlAnchor> clientAnchorElement = page.getAnchors().stream()
.filter(e -> e.asText().equals(clientRegistration.getClientName())).findFirst();

return (clientAnchorElement.orElse(null));
HtmlAnchor result = null;
for (HtmlAnchor anchor: page.getAnchors()) {
if (anchor.asText().equals(clientRegistration.getClientName())) {
result = anchor;
break;
}
}
return result;
}

private WebResponse followLinkDisableRedirects(HtmlAnchor anchorElement) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-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,7 +16,6 @@

package org.springframework.security.web.server.csrf;

import java.util.Optional;
import java.util.UUID;

import org.springframework.http.HttpCookie;
Expand Down Expand Up @@ -69,14 +68,17 @@ public Mono<CsrfToken> generateToken(ServerWebExchange exchange) {
@Override
public Mono<Void> saveToken(ServerWebExchange exchange, CsrfToken token) {
return Mono.fromRunnable(() -> {
Optional<String> tokenValue = Optional.ofNullable(token).map(CsrfToken::getToken);
String tokenValue = token != null ? token.getToken() : "";
int maxAge = !tokenValue.isEmpty() ? -1 : 0;
String path = this.cookiePath != null ? this.cookiePath : getRequestContext(exchange.getRequest());
boolean secure = exchange.getRequest().getSslInfo() != null;

ResponseCookie cookie = ResponseCookie.from(this.cookieName, tokenValue.orElse(""))
ResponseCookie cookie = ResponseCookie.from(this.cookieName, tokenValue)
.domain(this.cookieDomain)
.httpOnly(this.cookieHttpOnly)
.maxAge(tokenValue.map(val -> -1).orElse(0))
.path(Optional.ofNullable(this.cookiePath).orElseGet(() -> getRequestContext(exchange.getRequest())))
.secure(Optional.ofNullable(exchange.getRequest().getSslInfo()).map(sslInfo -> true).orElse(false))
.maxAge(maxAge)
.path(path)
.secure(secure)
.build();

exchange.getResponse().addCookie(cookie);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-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 @@ -17,7 +17,6 @@
package org.springframework.security.web.server.transport;

import java.net.URI;
import java.util.Optional;

import reactor.core.publisher.Mono;

Expand Down Expand Up @@ -102,10 +101,11 @@ private URI createRedirectUri(ServerWebExchange exchange) {
UriComponentsBuilder.fromUri(exchange.getRequest().getURI());

if (port > 0) {
Optional.ofNullable(this.portMapper.lookupHttpsPort(port))
.map(builder::port)
.orElseThrow(() -> new IllegalStateException(
"HTTP Port '" + port + "' does not have a corresponding HTTPS Port"));
Integer httpsPort = this.portMapper.lookupHttpsPort(port);
if (httpsPort == null) {
throw new IllegalStateException("HTTP Port '" + port + "' does not have a corresponding HTTPS Port");
}
builder.port(httpsPort);
}

return builder.scheme("https").build().toUri();
Expand Down