You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by cs...@apache.org on 2017/04/21 10:28:51 UTC

[3/6] cxf git commit: Blocking anonymous dynamic client reg even though it is allowed by the spec, generating client sec for other grants which require it

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/jms-exception-handling
Commit: ae1842229fbc354e57ca3ca6797e8c9462dfc2ce
Parents: 0b7b183
Author: Sergey Beryozkin <sb...@gmail.com>
Authored: Thu Apr 20 15:24:42 2017 +0100
Committer: Sergey Beryozkin <sb...@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;
+    }
+
 }