Repository: cxf Updated Branches: refs/heads/3.1.x-fixes 1e99be660 -> b3c4e25b8
Restricting 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/b3c4e25b Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/b3c4e25b Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/b3c4e25b Branch: refs/heads/3.1.x-fixes Commit: b3c4e25b8952b64f25fcc2ea1264b95e08e3a717 Parents: 1e99be6 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 14:01:46 2017 +0100 ---------------------------------------------------------------------- .../oauth2/client/OAuthClientUtils.java | 1 - .../oauth2/services/AbstractOAuthService.java | 12 ++++++++++ .../oauth2/services/AbstractTokenService.java | 25 ++++++++++++++++---- .../oauth2/common/OAuthDataProviderImpl.java | 2 ++ .../security/oauth2/grants/JAXRSOAuth2Test.java | 12 +++++++++- 5 files changed, 45 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/b3c4e25b/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 3f42117..a1467ba 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 */ http://git-wip-us.apache.org/repos/asf/cxf/blob/b3c4e25b/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 a3791f2..2d041ad 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 @@ -98,6 +98,18 @@ public abstract class AbstractOAuthService { 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. * By default this method will issue a warning for open http://git-wip-us.apache.org/repos/asf/cxf/blob/b3c4e25b/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 82e3502..e8df855 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/b3c4e25b/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 75e7d17..61e70f0 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/b3c4e25b/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 d05d3bb..33c9c46 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()); }