Repository: cxf Updated Branches: refs/heads/3.1.x-fixes 4774cc470 -> d7478d6ce
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/d7478d6c Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/d7478d6c Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/d7478d6c Branch: refs/heads/3.1.x-fixes Commit: d7478d6ce217c4843d10291ac32b586b72bceadf Parents: 4774cc4 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:36:19 2017 +0100 ---------------------------------------------------------------------- .../services/DynamicRegistrationService.java | 49 +++++++++++----- .../oidc/OIDCDynamicRegistrationTest.java | 62 ++++++++++---------- 2 files changed, 66 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/d7478d6c/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 3a6ed16..fab1886 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,28 +56,42 @@ 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); 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(); @@ -87,8 +101,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); } } @@ -206,21 +219,24 @@ public class DynamicRegistrationService { grantTypes = Collections.singletonList("authorization_code"); } - // Client Type + boolean passwordRequired = grantTypes.contains(OAuthConstants.AUTHORIZATION_CODE_GRANT) + || grantTypes.contains(OAuthConstants.RESOURCE_OWNER_GRANT) + || grantTypes.contains(OAuthConstants.CLIENT_CREDENTIALS_GRANT); + + // 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(); if (appType == null) { appType = DEFAULT_APPLICATION_TYPE; } - boolean isConfidential = DEFAULT_APPLICATION_TYPE.equals(appType) - && grantTypes.contains(OAuthConstants.AUTHORIZATION_CODE_GRANT); - + boolean isConfidential = DEFAULT_APPLICATION_TYPE.equals(appType) + && !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); @@ -304,7 +320,8 @@ public class DynamicRegistrationService { } protected String getRequestAccessToken() { - return AuthorizationUtils.getAuthorizationParts(getMessageContext(), + // This call will throw 401 if no given authorization scheme exists + return AuthorizationUtils.getAuthorizationParts(getMessageContext(), Collections.singleton(OAuthConstants.BEARER_AUTHORIZATION_SCHEME))[1]; } protected int getClientSecretSizeInBytes(ClientRegistration request) { @@ -323,4 +340,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/d7478d6c/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 9e772cf..6c9bfe2 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); - } - @org.junit.Test - public void testRegisterClientInitialAccessToken() throws Exception { - doTestRegisterClient("123456789"); + 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()); } - private void doTestRegisterClient(String initialAccessToken) throws Exception { + @org.junit.Test + 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"; - } - WebClient wc = WebClient.create(address, Collections.singletonList(new JsonMapObjectProvider()), + 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(), @@ -115,5 +103,17 @@ 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; + } + }
