[PKCE] allowpublic client option for grant_type authorization_code us…

…ing PKCE with S256 method (#1888) * [WIP] allowpublic client option corresponding to autoapprove Info: allowpublic is an optional flag similar to autoapprove to define behaviour in oauth2 flow. The option allow to omit client_secret parameter and/or client authentication in grant_type authorization_code. Escpecially mobile scenarios showed the need for such option, because other OpenID providers allow in meanwhile similar use cases. * support update of client * add runtime support for public client usage * formattings * formatting * add tests * sonar * tests * support update of client * Add example for login client allowpublic = true means, client_secret can be omitted * Update documentation * refactor grant_type check for public use and add more tests * version updated
cloudfoundry · Jun 21, 2022 · 306427a · 306427a
1 parent 3485d2b
commit 306427a
Show file tree

Hide file tree

Showing 19 changed files with 391 additions and 23 deletions.
diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/client/ClientConstants.java
@@ -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";

diff --git a/...ava/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java b/...ava/org/cloudfoundry/identity/uaa/authentication/ClientDetailsAuthenticationProvider.java
@@ -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 {
 
@@ -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;
@@ -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;
+    }
 }
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminBootstrap.java
@@ -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;
 
     /**
@@ -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,
@@ -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;
@@ -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();
     }
 
     /**
@@ -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) {

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java
@@ -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;
@@ -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;

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/UaaClient.java
@@ -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;
+  }
+
+}
diff --git a/...rc/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java b/...rc/main/java/org/cloudfoundry/identity/uaa/client/UaaClientDetailsUserDetailsService.java
@@ -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());
+  }
+
+}
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/UaaConfiguration.java b/server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/UaaConfiguration.java
@@ -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 {
@@ -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;

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/pkce/PkceValidationService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/pkce/PkceValidationService.java
@@ -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;
 
 /**
@@ -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.
@@ -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));
     }

diff --git a/.../org/cloudfoundry/identity/uaa/oauth/token/PkceEnhancedAuthorizationCodeTokenGranter.java b/.../org/cloudfoundry/identity/uaa/oauth/token/PkceEnhancedAuthorizationCodeTokenGranter.java
@@ -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);
             }