Repository: cxf
Updated Branches:
  refs/heads/master 6c28faf78 -> 514c06899


Retsricting the OAuth2 client auth method when possible


Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/514c0689
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/514c0689
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/514c0689

Branch: refs/heads/master
Commit: 514c06899fc56b622ec2a1117e857f0d5a8f45a6
Parents: 6c28faf
Author: Sergey Beryozkin <sberyoz...@gmail.com>
Authored: Fri Apr 28 13:55:12 2017 +0100
Committer: Sergey Beryozkin <sberyoz...@gmail.com>
Committed: Fri Apr 28 13:55:12 2017 +0100

----------------------------------------------------------------------
 .../oauth2/client/OAuthClientUtils.java         |  3 +--
 .../oauth2/services/AbstractOAuthService.java   | 12 ++++++++++
 .../oauth2/services/AbstractTokenService.java   | 25 ++++++++++++++++----
 .../oauth2/common/OAuthDataProviderImpl.java    |  2 ++
 .../security/oauth2/grants/JAXRSOAuth2Test.java | 12 +++++++++-
 5 files changed, 46 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/514c0689/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/client/OAuthClientUtils.java
----------------------------------------------------------------------
diff --git 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/client/OAuthClientUtils.java
 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/client/OAuthClientUtils.java
index b591041..5a41ebc 100644
--- 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/client/OAuthClientUtils.java
+++ 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/client/OAuthClientUtils.java
@@ -184,7 +184,6 @@ public final class OAuthClientUtils {
      * using the initialized web client
      * @param accessTokenService the AccessToken client
      * @param grant {@link AccessTokenGrant} grant
-     * @param extraParams extra parameters
      * @return {@link ClientAccessToken} access token
      * @throws OAuthServiceException
      */
@@ -193,7 +192,7 @@ public final class OAuthClientUtils {
         throws OAuthServiceException {
         return getAccessToken(accessTokenService, null, grant, null, false);
     }
-
+    
     /**
      * Obtains the access token from OAuth AccessToken Service
      * using the initialized web client

http://git-wip-us.apache.org/repos/asf/cxf/blob/514c0689/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractOAuthService.java
----------------------------------------------------------------------
diff --git 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractOAuthService.java
 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractOAuthService.java
index f64ce6e..63a9559 100644
--- 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractOAuthService.java
+++ 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractOAuthService.java
@@ -97,6 +97,18 @@ public abstract class AbstractOAuthService {
         LOG.fine("No valid client found as the given clientId is null");
         return null;
     }
+    
+    protected Client getValidClient(String clientId, String clientSecret, 
MultivaluedMap<String, String> params)
+        throws OAuthServiceException {
+        if (clientId != null) {
+            mc.put(OAuthConstants.CLIENT_SECRET, clientSecret);
+            mc.put(OAuthConstants.GRANT_TYPE, 
params.getFirst(OAuthConstants.GRANT_TYPE));
+            mc.put(OAuthConstants.TOKEN_REQUEST_PARAMS, params);
+            return dataProvider.getClient(clientId);
+        }
+        LOG.fine("No valid client found as the given clientId is null");
+        return null;
+    }
 
     /**
      * HTTPS is the default transport for OAuth 2.0 services.

http://git-wip-us.apache.org/repos/asf/cxf/blob/514c0689/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractTokenService.java
----------------------------------------------------------------------
diff --git 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractTokenService.java
 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractTokenService.java
index 826e86d..1657d59 100644
--- 
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractTokenService.java
+++ 
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/AbstractTokenService.java
@@ -62,9 +62,11 @@ public class AbstractTokenService extends 
AbstractOAuthService {
                 String clientSecret = 
params.getFirst(OAuthConstants.CLIENT_SECRET);
                 if (clientSecret != null) {
                     client = getAndValidateClientFromIdAndSecret(clientId, 
clientSecret, params);
+                    validateClientAuthenticationMethod(client, 
OAuthConstants.TOKEN_ENDPOINT_AUTH_POST);
                 } else if (OAuthUtils.isMutualTls(sc, getTlsSessionInfo())) {
                     client = getClient(clientId, params);
                     checkCertificateBinding(client, getTlsSessionInfo());
+                    validateClientAuthenticationMethod(client, 
OAuthConstants.TOKEN_ENDPOINT_AUTH_TLS);
                 }
             }
         } else {
@@ -93,6 +95,13 @@ public class AbstractTokenService extends 
AbstractOAuthService {
         return client;
     }
 
+    protected void validateClientAuthenticationMethod(Client c, String 
authMethod) {
+        if (c != null && c.getTokenEndpointAuthMethod() != null
+            && !c.getTokenEndpointAuthMethod().equals(authMethod)) {
+            reportInvalidClient(new 
OAuthError(OAuthConstants.UNAUTHORIZED_CLIENT));
+        }
+    }
+    
     protected String retrieveClientId(MultivaluedMap<String, String> params) {
         String clientId = params.getFirst(OAuthConstants.CLIENT_ID);
         if (clientId == null) {
@@ -108,7 +117,7 @@ public class AbstractTokenService extends 
AbstractOAuthService {
     protected Client getAndValidateClientFromIdAndSecret(String clientId,
                                                          String 
providedClientSecret,
                                                          
MultivaluedMap<String, String> params) {
-        Client client = getClient(clientId, params);
+        Client client = getClient(clientId, providedClientSecret, params);
         if (!client.getClientId().equals(clientId)) {
             reportInvalidClient();
         }
@@ -137,12 +146,13 @@ public class AbstractTokenService extends 
AbstractOAuthService {
     }
 
     protected Client getClientFromBasicAuthScheme(MultivaluedMap<String, 
String> params) {
+        Client client = null;
         String[] userInfo = 
AuthorizationUtils.getBasicAuthUserInfo(getMessageContext());
         if (userInfo != null && userInfo.length == 2) {
-            return getAndValidateClientFromIdAndSecret(userInfo[0], 
userInfo[1], params);
-        } else {
-            return null;
+            client = getAndValidateClientFromIdAndSecret(userInfo[0], 
userInfo[1], params);
         }
+        validateClientAuthenticationMethod(client, 
OAuthConstants.TOKEN_ENDPOINT_AUTH_BASIC);
+        return client;
     }
 
     protected void checkCertificateBinding(Client client, TLSSessionInfo 
tlsSessionInfo) {
@@ -185,6 +195,7 @@ public class AbstractTokenService extends 
AbstractOAuthService {
             String subjectDn = 
OAuthUtils.getSubjectDnFromTLSCertificates(cert);
             if (!StringUtils.isEmpty(subjectDn)) {
                 client = getClient(subjectDn, params);
+                validateClientAuthenticationMethod(client, 
OAuthConstants.TOKEN_ENDPOINT_AUTH_TLS);
                 // The certificates must be registered with the client and 
match TLS certificates
                 // in case of the binding where Client's clientId is a subject 
distinguished name
                 compareTlsCertificates(tlsSessionInfo, 
client.getApplicationCertificates());
@@ -230,13 +241,17 @@ public class AbstractTokenService extends 
AbstractOAuthService {
      * @throws {@link javax.ws.rs.WebApplicationException} if no matching 
Client is found
      */
     protected Client getClient(String clientId, MultivaluedMap<String, String> 
params) {
+        return getClient(clientId, 
params.getFirst(OAuthConstants.CLIENT_SECRET), params);
+    }
+    
+    protected Client getClient(String clientId, String clientSecret, 
MultivaluedMap<String, String> params) {
         if (clientId == null) {
             reportInvalidRequestError("Client ID is null");
             return null;
         }
         Client client = null;
         try {
-            client = getValidClient(clientId, params);
+            client = getValidClient(clientId, clientSecret, params);
         } catch (OAuthServiceException ex) {
             LOG.warning("No valid client found for clientId: " + clientId);
             if (ex.getError() != null) {

http://git-wip-us.apache.org/repos/asf/cxf/blob/514c0689/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/common/OAuthDataProviderImpl.java
----------------------------------------------------------------------
diff --git 
a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/common/OAuthDataProviderImpl.java
 
b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/common/OAuthDataProviderImpl.java
index 2b6dc8b..a495d0b 100644
--- 
a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/common/OAuthDataProviderImpl.java
+++ 
b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/common/OAuthDataProviderImpl.java
@@ -32,6 +32,7 @@ import 
org.apache.cxf.rs.security.oauth2.common.OAuthPermission;
 import 
org.apache.cxf.rs.security.oauth2.grants.code.DefaultEHCacheCodeDataProvider;
 import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException;
 import org.apache.cxf.rs.security.oauth2.saml.Constants;
+import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants;
 import org.apache.cxf.rt.security.crypto.CryptoUtils;
 import org.apache.xml.security.utils.ClassLoaderUtils;
 
@@ -140,6 +141,7 @@ public class OAuthDataProviderImpl extends 
DefaultEHCacheCodeDataProvider {
             String clientSecret = super.getCurrentClientSecret();
             if (externalClients.contains(clientId + ":" + clientSecret)) {
                 c = new Client(clientId, clientSecret, true);
+                
c.setTokenEndpointAuthMethod(OAuthConstants.TOKEN_ENDPOINT_AUTH_BASIC);
             }
         }
         return c;

http://git-wip-us.apache.org/repos/asf/cxf/blob/514c0689/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/grants/JAXRSOAuth2Test.java
----------------------------------------------------------------------
diff --git 
a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/grants/JAXRSOAuth2Test.java
 
b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/grants/JAXRSOAuth2Test.java
index 901063a..3acbf51 100644
--- 
a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/grants/JAXRSOAuth2Test.java
+++ 
b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oauth2/grants/JAXRSOAuth2Test.java
@@ -148,7 +148,17 @@ public class JAXRSOAuth2Test extends 
AbstractBusClientServerTestBase {
         // (instead WebClient can be initialized with username & password)
         grant.setClientId("bob");
         grant.setClientSecret("bobPassword");
-        ClientAccessToken at = OAuthClientUtils.getAccessToken(wc, grant);
+        try {
+            OAuthClientUtils.getAccessToken(wc, grant);
+            fail("Form based authentication is not supported");
+        } catch (OAuthServiceException ex) {
+            assertEquals(OAuthConstants.UNAUTHORIZED_CLIENT, 
ex.getError().getError());
+        }
+        
+        ClientAccessToken at = OAuthClientUtils.getAccessToken(wc,  
+                                                               new 
Consumer("bob", "bobPassword"),
+                                                               new 
ClientCredentialsGrant(),
+                                                               true);
         assertNotNull(at.getTokenKey());
     }
 

Reply via email to