Skip to content
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.

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

[PKCE] allowpublic client option for grant_type authorization_code using PKCE with S256 method #1888

Merged
merged 15 commits into from
Jun 21, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
public class ClientConstants {
public static final String ALLOWED_PROVIDERS = "allowedproviders";
public static final String AUTO_APPROVE = "autoapprove";
public static final String ALLOW_PUBLIC = "allowpublic";
public static final String CREATED_WITH = "createdwith";
public static final String CLIENT_NAME = "name";
public static final String APPROVALS_DELETED = "approvals_deleted";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,23 @@
*******************************************************************************/
package org.cloudfoundry.identity.uaa.authentication;

import org.apache.directory.api.util.Strings;
import org.cloudfoundry.identity.uaa.client.UaaClient;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.oauth.pkce.PkceValidationService;
import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.util.StringUtils;

import java.util.Collections;
import java.util.Map;

public class ClientDetailsAuthenticationProvider extends DaoAuthenticationProvider {

Expand All @@ -45,6 +55,15 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
for(String pwd: passwordList) {
try {
User user = new User(userDetails.getUsername(), pwd, userDetails.isEnabled(), userDetails.isAccountNonExpired(), userDetails.isCredentialsNonExpired(), userDetails.isAccountNonLocked(), userDetails.getAuthorities());
if (authentication.getCredentials() == null && isGrantAuthorizationCode(authentication.getDetails()) && userDetails instanceof UaaClient) {
// in case of grant_type=authorization_code and code_verifier passed (PKCE) we check if client has option allowpublic with true and proceed even if no secret is provided
UaaClient uaaClient = (UaaClient) userDetails;
Object allowPublic = uaaClient.getAdditionalInformation().get(ClientConstants.ALLOW_PUBLIC);
if (allowPublic instanceof String && Boolean.TRUE.toString().equalsIgnoreCase((String)allowPublic) ||
allowPublic instanceof Boolean && Boolean.TRUE.equals(allowPublic)) {
break;
}
}
super.additionalAuthenticationChecks(user, authentication);
error = null;
break;
Expand All @@ -56,4 +75,22 @@ protected void additionalAuthenticationChecks(UserDetails userDetails, UsernameP
throw error;
}
}

private boolean isGrantAuthorizationCode(Object uaaAuthenticationDetails) {
Map<String, String[]> requestParameters = uaaAuthenticationDetails instanceof UaaAuthenticationDetails &&
((UaaAuthenticationDetails)uaaAuthenticationDetails).getParameterMap() != null ?
((UaaAuthenticationDetails)uaaAuthenticationDetails).getParameterMap() : Collections.emptyMap();
return PkceValidationService.isCodeVerifierParameterValid(getSafeParameterValue(requestParameters.get("code_verifier"))) &&
StringUtils.hasText(getSafeParameterValue(requestParameters.get("client_id"))) &&
StringUtils.hasText(getSafeParameterValue(requestParameters.get("code"))) &&
StringUtils.hasText(getSafeParameterValue(requestParameters.get("redirect_uri"))) &&
TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE.equals(getSafeParameterValue(requestParameters.get(ClaimConstants.GRANT_TYPE)));
}

private String getSafeParameterValue(String[] value) {
if (null == value || value.length < 1) {
return Strings.EMPTY_STRING;
}
return StringUtils.hasText(value[0]) ? value[0] : Strings.EMPTY_STRING;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class ClientAdminBootstrap implements
private final Set<String> clientsToDelete;
private final JdbcTemplate jdbcTemplate;
private final Set<String> autoApproveClients;
private final Set<String> allowPublicClients;
private final boolean defaultOverride;

/**
Expand All @@ -70,6 +71,8 @@ public class ClientAdminBootstrap implements
* client details map). These clients will have
* <code>autoapprove=true</code> when they are inserted
* into the client details store.
* @param allowPublicClients A set of client ids that are allowed to be used
* without client_secret parameter but with PKCE S256 method
*/
ClientAdminBootstrap(
final PasswordEncoder passwordEncoder,
Expand All @@ -79,7 +82,8 @@ public class ClientAdminBootstrap implements
final Map<String, Map<String, Object>> clients,
final Collection<String> autoApproveClients,
final Collection<String> clientsToDelete,
final JdbcTemplate jdbcTemplate) {
final JdbcTemplate jdbcTemplate,
final Set<String> allowPublicClients) {
this.passwordEncoder = passwordEncoder;
this.clientRegistrationService = clientRegistrationService;
this.clientMetadataProvisioning = clientMetadataProvisioning;
Expand All @@ -88,12 +92,14 @@ public class ClientAdminBootstrap implements
this.autoApproveClients = new HashSet<>(ofNullable(autoApproveClients).orElse(Collections.emptySet()));
this.clientsToDelete = new HashSet<>(ofNullable(clientsToDelete).orElse(Collections.emptySet()));
this.jdbcTemplate = jdbcTemplate;
this.allowPublicClients = new HashSet<>(ofNullable(allowPublicClients).orElse(Collections.emptySet()));
}

@Override
public void afterPropertiesSet() {
addNewClients();
updateAutoApproveClients();
updateAllowedPublicClients();
}

/**
Expand All @@ -114,6 +120,20 @@ private void updateAutoApproveClients() {
}
}

private void updateAllowedPublicClients() {
allowPublicClients.removeAll(clientsToDelete);
for (String clientId : allowPublicClients) {
try {
BaseClientDetails base = (BaseClientDetails) clientRegistrationService.loadClientByClientId(clientId, IdentityZone.getUaaZoneId());
base.addAdditionalInformation(ClientConstants.ALLOW_PUBLIC, true);
logger.debug("Adding allowpublic flag to client: {}", clientId);
clientRegistrationService.updateClientDetails(base, IdentityZone.getUaaZoneId());
} catch (NoSuchClientException n) {
logger.debug("Client not found, unable to set allowpublic: {}", clientId);
}
}
}

private String getRedirectUris(Map<String, Object> map) {
Set<String> redirectUris = new HashSet<>();
if (map.get("redirect-uri") != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
import org.cloudfoundry.identity.uaa.client.ClientDetailsValidator.Mode;
import org.cloudfoundry.identity.uaa.error.UaaException;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsCreation;
import org.cloudfoundry.identity.uaa.oauth.client.ClientDetailsModification;
import org.cloudfoundry.identity.uaa.oauth.client.SecretChangeRequest;
Expand Down Expand Up @@ -674,6 +675,9 @@ private ClientDetails syncWithExisting(ClientDetails existing, ClientDetails inp
additionalInformation.remove(key);
}
}
if (Boolean.FALSE.equals(additionalInformation.get(ClientConstants.ALLOW_PUBLIC))) {
additionalInformation.remove(ClientConstants.ALLOW_PUBLIC);
}
details.setAdditionalInformation(additionalInformation);

return details;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package org.cloudfoundry.identity.uaa.client;

import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.SpringSecurityCoreVersion;
import org.springframework.security.core.userdetails.User;

import java.util.Collection;
import java.util.Map;

public class UaaClient extends User {

private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID;
private transient Map<String, Object> additionalInformation;

public UaaClient(String username, String password, Collection<? extends GrantedAuthority> authorities, Map<String, Object> additionalInformation) {
super(username, password, authorities);
this.additionalInformation = additionalInformation;
}

public Map<String, Object> getAdditionalInformation() {
return this.additionalInformation;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.cloudfoundry.identity.uaa.client;

import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.security.oauth2.provider.NoSuchClientException;

public class UaaClientDetailsUserDetailsService implements UserDetailsService {

private final ClientDetailsService clientDetailsService;
private String emptyPassword = "";

public UaaClientDetailsUserDetailsService(final ClientDetailsService clientDetailsService) {
this.clientDetailsService = clientDetailsService;
}

/**
* @param passwordEncoder the password encoder to set
*/
public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
this.emptyPassword = passwordEncoder.encode("");
}

public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
ClientDetails clientDetails;
try {
clientDetails = clientDetailsService.loadClientByClientId(username);
} catch (NoSuchClientException e) {
throw new UsernameNotFoundException(e.getMessage(), e);
}
String clientSecret = clientDetails.getClientSecret();
if (clientSecret== null || clientSecret.trim().length()==0) {
clientSecret = emptyPassword;
}
return new UaaClient(username, clientSecret, clientDetails.getAuthorities(), clientDetails.getAdditionalInformation());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ public static class OAuth {
public static class Client {
public String override;
public List<String> autoapprove;
public List<String> allowpublic;
}

public static class Authorize {
Expand All @@ -210,6 +211,7 @@ public static class OAuthClient {
public String id;
public boolean override;
public List<String> autoapprove;
public List<String> allowpublic;
public String scope;
public String secret;
public String authorities;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package org.cloudfoundry.identity.uaa.oauth.pkce;

import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.springframework.security.oauth2.common.exceptions.InvalidGrantException;
import org.springframework.security.oauth2.provider.ClientDetails;

import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -70,6 +73,8 @@ public boolean isCodeChallengeMethodSupported(String codeChallengeMethod) {
* Map of query parameters of Authorization request.
* @param codeVerifier
* Code verifier.
* @param clientDetails
* Allow public information from client
* @return true: (1) in case of Authorization Code Grant without PKCE.
* (2) in case of Authorization Code Grant with PKCE and code verifier
* matched with code challenge based on code challenge method.
Expand All @@ -79,11 +84,15 @@ public boolean isCodeChallengeMethodSupported(String codeChallengeMethod) {
* (1) Code verifier must be provided for this authorization code.
* (2) Code verifier not required for this authorization code.
*/
public boolean checkAndValidate(Map<String, String> requestParameters, String codeVerifier) throws PkceValidationException {
public boolean checkAndValidate(Map<String, String> requestParameters, String codeVerifier, ClientDetails clientDetails) throws PkceValidationException {
if (!hasPkceParameters(requestParameters, codeVerifier)) {
return true;
}
String codeChallengeMethod = extractCodeChallengeMethod(requestParameters);
if (!"S256".equalsIgnoreCase(codeChallengeMethod) && clientDetails != null && clientDetails.getAdditionalInformation() != null
&& clientDetails.getAdditionalInformation().get(ClientConstants.ALLOW_PUBLIC) != null) {
throw new InvalidGrantException("Public client must use code challange method S256");
}
return pkceVerifiers.get(codeChallengeMethod).verify(codeVerifier,
requestParameters.get(PkceValidationService.CODE_CHALLENGE));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ protected OAuth2Authentication getOAuth2Authentication(ClientDetails client, Tok
* PKCE code verifier parameter verification
*/
try {
if (pkceValidationService != null && !pkceValidationService.checkAndValidate(storedAuth.getOAuth2Request().getRequestParameters(), codeVerifier)) {
if ( pkceValidationService != null &&
!pkceValidationService.checkAndValidate(storedAuth.getOAuth2Request().getRequestParameters(), codeVerifier, client)) {
// has PkceValidation service and validation failed
throw new InvalidGrantException("Invalid code verifier: " + codeVerifier);
}
Expand Down
Loading