Repository: cxf Updated Branches: refs/heads/master a98345918 -> c0efcafea
Blocking anonymous dynamic client reg even though it is allowed by the spec, generating client sec for other grants which require it Project: http://git-wip-us.apache.org/repos/asf/cxf/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/ae184222 Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/ae184222 Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/ae184222 Branch: refs/heads/master Commit: ae1842229fbc354e57ca3ca6797e8c9462dfc2ce Parents: 0b7b183 Author: Sergey Beryozkin <sberyoz...@gmail.com> Authored: Thu Apr 20 15:24:42 2017 +0100 Committer: Sergey Beryozkin <sberyoz...@gmail.com> Committed: Thu Apr 20 15:24:42 2017 +0100 ---------------------------------------------------------------------- .../services/DynamicRegistrationService.java | 38 ++++++++++--- .../oidc/OIDCDynamicRegistrationTest.java | 58 ++++++++++---------- 2 files changed, 59 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/ae184222/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/DynamicRegistrationService.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/DynamicRegistrationService.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/DynamicRegistrationService.java index 7af9993..7f914ec 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/DynamicRegistrationService.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/DynamicRegistrationService.java @@ -56,12 +56,13 @@ public class DynamicRegistrationService { private int clientIdSizeInBytes = DEFAULT_CLIENT_ID_SIZE; private MessageContext mc; private boolean supportRegistrationAccessTokens = true; + private String userRole; @POST @Consumes("application/json") @Produces("application/json") public Response register(ClientRegistration request) { - checkInitialAccessToken(); + checkInitialAuthentication(); Client client = createNewClient(request); createRegAccessToken(client); clientProvider.setClient(client); @@ -69,15 +70,28 @@ public class DynamicRegistrationService { return Response.status(201).entity(fromClientToRegistrationResponse(client)).build(); } - protected void checkInitialAccessToken() { + protected void checkInitialAuthentication() { if (initialAccessToken != null) { String accessToken = getRequestAccessToken(); if (!initialAccessToken.equals(accessToken)) { throw ExceptionUtils.toNotAuthorizedException(null, null); } + } else { + checkSecurityContext(); } } + + + protected void checkSecurityContext() { + SecurityContext sc = mc.getSecurityContext(); + if (sc.getUserPrincipal() == null) { + throw ExceptionUtils.toNotAuthorizedException(null, null); + } + if (userRole != null && !sc.isUserInRole(userRole)) { + throw ExceptionUtils.toForbiddenException(null, null); + } + } protected String createRegAccessToken(Client client) { String regAccessToken = OAuthUtils.generateRandomTokenKey(); @@ -88,7 +102,7 @@ public class DynamicRegistrationService { protected void checkRegistrationAccessToken(Client c, String accessToken) { String regAccessToken = c.getProperties().get(ClientRegistrationResponse.REG_ACCESS_TOKEN); - if (!regAccessToken.equals(accessToken)) { + if (regAccessToken == null || !regAccessToken.equals(accessToken)) { throw ExceptionUtils.toNotAuthorizedException(null, null); } } @@ -205,8 +219,12 @@ public class DynamicRegistrationService { if (grantTypes == null) { grantTypes = Collections.singletonList("authorization_code"); } + + boolean passwordRequired = grantTypes.contains(OAuthConstants.AUTHORIZATION_CODE_GRANT) + || grantTypes.contains(OAuthConstants.RESOURCE_OWNER_GRANT) + || grantTypes.contains(OAuthConstants.CLIENT_CREDENTIALS_GRANT); - // Client Type + // Application Type // https://tools.ietf.org/html/rfc7591 has no this property but // but http://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata does String appType = request.getApplicationType(); @@ -214,13 +232,12 @@ public class DynamicRegistrationService { appType = DEFAULT_APPLICATION_TYPE; } boolean isConfidential = DEFAULT_APPLICATION_TYPE.equals(appType) - && grantTypes.contains(OAuthConstants.AUTHORIZATION_CODE_GRANT); + && !grantTypes.contains(OAuthConstants.IMPLICIT_GRANT); // Client Secret - String clientSecret = isConfidential - ? generateClientSecret(request) - : null; + String clientSecret = passwordRequired ? generateClientSecret(request) : null; + Client newClient = new Client(clientId, clientSecret, isConfidential, clientName); newClient.setAllowedGrantTypes(grantTypes); @@ -305,6 +322,7 @@ public class DynamicRegistrationService { } protected String getRequestAccessToken() { + // This call will throw 401 if no given authorization scheme exists return AuthorizationUtils.getAuthorizationParts(getMessageContext(), Collections.singleton(OAuthConstants.BEARER_AUTHORIZATION_SCHEME))[1]; } @@ -324,4 +342,8 @@ public class DynamicRegistrationService { public void setSupportRegistrationAccessTokens(boolean supportRegistrationAccessTokens) { this.supportRegistrationAccessTokens = supportRegistrationAccessTokens; } + + public void setUserRole(String userRole) { + this.userRole = userRole; + } } http://git-wip-us.apache.org/repos/asf/cxf/blob/ae184222/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCDynamicRegistrationTest.java ---------------------------------------------------------------------- diff --git a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCDynamicRegistrationTest.java b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCDynamicRegistrationTest.java index abc166f..22b97a2 100644 --- a/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCDynamicRegistrationTest.java +++ b/systests/rs-security/src/test/java/org/apache/cxf/systest/jaxrs/security/oidc/OIDCDynamicRegistrationTest.java @@ -52,43 +52,31 @@ public class OIDCDynamicRegistrationTest extends AbstractBusClientServerTestBase assertEquals(401, r.getStatus()); } @org.junit.Test - public void testRegisterClient() throws Exception { - doTestRegisterClient(null); + public void testRegisterClientNoInitialAccessToken() throws Exception { + URL busFile = OIDCDynamicRegistrationTest.class.getResource("client.xml"); + String address = "https://localhost:" + PORT + "/services/dynamic/register"; + WebClient wc = WebClient.create(address, Collections.singletonList(new JsonMapObjectProvider()), + busFile.toString()); + wc.accept("application/json").type("application/json"); + + assertEquals(401, wc.post(newClientRegistration()).getStatus()); } + @org.junit.Test - public void testRegisterClientInitialAccessToken() throws Exception { - doTestRegisterClient("123456789"); - } - - private void doTestRegisterClient(String initialAccessToken) throws Exception { + public void testRegisterClientInitialAccessTokenCodeGrant() throws Exception { URL busFile = OIDCDynamicRegistrationTest.class.getResource("client.xml"); - String address = "https://localhost:" + PORT + "/services"; - if (initialAccessToken != null) { - address = address + "/dynamicWithAt/register"; - } else { - address = address + "/dynamic/register"; - } + String address = "https://localhost:" + PORT + "/services/dynamicWithAt/register"; WebClient wc = WebClient.create(address, Collections.singletonList(new JsonMapObjectProvider()), busFile.toString()); wc.accept("application/json").type("application/json"); - ClientRegistration reg = new ClientRegistration(); - reg.setApplicationType("web"); - reg.setScope("openid"); - reg.setClientName("dynamic_client"); - reg.setGrantTypes(Collections.singletonList("authorization_code")); - reg.setRedirectUris(Collections.singletonList("https://a/b/c")); - reg.setProperty("post_logout_redirect_uris", - Collections.singletonList("https://rp/logout")); + ClientRegistration reg = newClientRegistration(); ClientRegistrationResponse resp = null; - Response r = wc.post(reg); - if (initialAccessToken == null) { - resp = r.readEntity(ClientRegistrationResponse.class); - } else { - assertEquals(401, wc.get().getStatus()); - wc.authorization(new ClientAccessToken("Bearer", initialAccessToken)); - resp = wc.post(reg, ClientRegistrationResponse.class); - } + assertEquals(401, wc.post(reg).getStatus()); + + wc.authorization(new ClientAccessToken("Bearer", "123456789")); + resp = wc.post(reg, ClientRegistrationResponse.class); + assertNotNull(resp.getClientId()); assertNotNull(resp.getClientSecret()); assertEquals(address + "/" + resp.getClientId(), @@ -116,4 +104,16 @@ public class OIDCDynamicRegistrationTest extends AbstractBusClientServerTestBase assertEquals(200, wc.delete().getStatus()); } + private ClientRegistration newClientRegistration() { + ClientRegistration reg = new ClientRegistration(); + reg.setApplicationType("web"); + reg.setScope("openid"); + reg.setClientName("dynamic_client"); + reg.setGrantTypes(Collections.singletonList("authorization_code")); + reg.setRedirectUris(Collections.singletonList("https://a/b/c")); + reg.setProperty("post_logout_redirect_uris", + Collections.singletonList("https://rp/logout")); + return reg; + } + }