Merge pull request #3081 from pedroigor/master

[KEYCLOAK-3338] More testing and improvements when importing role poicies

examples/authz/photoz/photoz-realm.json

@@ -22,7 +22,12 @@
       ],
       "realmRoles": [
         "user", "uma_authorization"
-      ]
+      ],
+      "clientRoles": {
+        "photoz-restful-api": [
+          "manage-albums"
+        ]
+      }
     },
     {
       "username": "jdoe",
@@ -38,7 +43,12 @@
       ],
       "realmRoles": [
         "user", "uma_authorization"
-      ]
+      ],
+      "clientRoles": {
+        "photoz-restful-api": [
+          "manage-albums"
+        ]
+      }
     },
     {
       "username": "admin",
@@ -58,6 +68,9 @@
       "clientRoles": {
         "realm-management": [
           "realm-admin"
+        ],
+        "photoz-restful-api": [
+          "manage-albums"
         ]
       }
     },
@@ -90,6 +103,8 @@
       "adminUrl": "/photoz-html5-client",
       "baseUrl": "/photoz-html5-client",
       "publicClient": true,
+      "consentRequired" : true,
+      "fullScopeAllowed" : true,
       "redirectUris": [
         "/photoz-html5-client/*"
       ],

examples/authz/photoz/photoz-restful-api-authz-service.json

@@ -70,13 +70,13 @@
     },
     {
       "name": "Any User Policy",
-      "description": "Defines that any user can do something",
+      "description": "Defines that only users from well known clients are allowed to access",
       "type": "role",
       "logic": "POSITIVE",
       "decisionStrategy": "UNANIMOUS",
       "config": {
         "applyPolicies": "[]",
-        "roles": "[{\"id\":\"user\"}]"
+        "roles": "[{\"id\":\"user\"},{\"id\":\"manage-albums\",\"required\":true}]"
       }
     },
     {
@@ -97,7 +97,7 @@
       "logic": "POSITIVE",
       "decisionStrategy": "UNANIMOUS",
       "config": {
-        "applyPolicies": "[\"Any Admin Policy\",\"Only From a Specific Client Address\"]"
+        "applyPolicies": "[\"Only From a Specific Client Address\",\"Any Admin Policy\"]"
       }
     },
     {
@@ -107,7 +107,7 @@
       "logic": "POSITIVE",
       "decisionStrategy": "AFFIRMATIVE",
       "config": {
-        "applyPolicies": "[\"Only Owner Policy\",\"Administration Policy\"]"
+        "applyPolicies": "[\"Administration Policy\",\"Only Owner Policy\"]"
       }
     },
     {

services/src/main/java/org/keycloak/authorization/admin/ResourceServerService.java

@@ -45,6 +45,7 @@ import org.keycloak.representations.idm.authorization.ScopeRepresentation;
 import org.keycloak.services.resources.admin.RealmAuth;
 import org.keycloak.util.JsonSerialization;
 
+import javax.management.relation.Role;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
@@ -61,6 +62,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
@@ -256,7 +259,35 @@ public class ResourceServerService {
                 try {
                     List<Map> rolesMap = JsonSerialization.readValue(roles, List.class);
                     config.put("roles", JsonSerialization.writeValueAsString(rolesMap.stream().map(roleConfig -> {
-                        roleConfig.put("id", realm.getRole(roleConfig.get("id").toString()).getId());
+                        String roleName = roleConfig.get("id").toString();
+                        String clientId = null;
+                        int clientIdSeparator = roleName.indexOf("/");
+
+                        if (clientIdSeparator != -1) {
+                            clientId = roleName.substring(0, clientIdSeparator);
+                            roleName = roleName.substring(clientIdSeparator + 1);
+                        }
+
+                        RoleModel role;
+
+                        if (clientId == null) {
+                            role = realm.getRole(roleName);
+                        } else {
+                            role = realm.getClientByClientId(clientId).getRole(roleName);
+                        }
+
+                        // fallback to find any client role with the given name
+                        if (role == null) {
+                            String finalRoleName = roleName;
+                            role = realm.getClients().stream().map(clientModel -> clientModel.getRole(finalRoleName)).filter(roleModel -> roleModel != null)
+                                    .findFirst().orElse(null);
+                        }
+
+                        if (role == null) {
+                            throw new RuntimeException("Error while importing configuration. Role [" + role + "] could not be found.");
+                        }
+
+                        roleConfig.put("id", role.getId());
                         return roleConfig;
                     }).collect(Collectors.toList())));
                 } catch (Exception e) {

testsuite/integration-arquillian/test-apps/photoz/photoz-realm.json

@@ -22,7 +22,12 @@
       ],
       "realmRoles": [
         "user", "uma_authorization"
-      ]
+      ],
+      "clientRoles": {
+        "photoz-restful-api": [
+          "manage-albums"
+        ]
+      }
     },
     {
       "username": "jdoe",
@@ -38,7 +43,12 @@
       ],
       "realmRoles": [
         "user", "uma_authorization"
-      ]
+      ],
+      "clientRoles": {
+        "photoz-restful-api": [
+          "manage-albums"
+        ]
+      }
     },
     {
       "username": "admin",
@@ -58,6 +68,9 @@
       "clientRoles": {
         "realm-management": [
           "realm-admin"
+        ],
+        "photoz-restful-api": [
+          "manage-albums"
         ]
       }
     },
@@ -90,6 +103,8 @@
       "adminUrl": "/photoz-html5-client",
       "baseUrl": "/photoz-html5-client",
       "publicClient": true,
+      "consentRequired" : true,
+      "fullScopeAllowed" : true,
       "redirectUris": [
         "/photoz-html5-client/*"
       ],

testsuite/integration-arquillian/test-apps/photoz/photoz-restful-api-authz-service.json

@@ -70,13 +70,13 @@
     },
     {
       "name": "Any User Policy",
-      "description": "Defines that any user can do something",
+      "description": "Defines that only users from well known clients are allowed to access",
       "type": "role",
       "logic": "POSITIVE",
       "decisionStrategy": "UNANIMOUS",
       "config": {
         "applyPolicies": "[]",
-        "roles": "[{\"id\":\"user\"}]"
+        "roles": "[{\"id\":\"user\"},{\"id\":\"manage-albums\",\"required\":true}]"
       }
     },
     {
@@ -97,7 +97,7 @@
       "logic": "POSITIVE",
       "decisionStrategy": "UNANIMOUS",
       "config": {
-        "applyPolicies": "[\"Any Admin Policy\",\"Only From a Specific Client Address\"]"
+        "applyPolicies": "[\"Only From a Specific Client Address\",\"Any Admin Policy\"]"
       }
     },
     {
@@ -107,7 +107,7 @@
       "logic": "POSITIVE",
       "decisionStrategy": "AFFIRMATIVE",
       "config": {
-        "applyPolicies": "[\"Only Owner Policy\",\"Administration Policy\"]"
+        "applyPolicies": "[\"Administration Policy\",\"Only Owner Policy\"]"
       }
     },
     {

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/PhotozClientAuthzTestApp.java

@@ -22,11 +22,13 @@ import org.jboss.arquillian.test.api.ArquillianResource;
 import org.keycloak.testsuite.auth.page.login.OIDCLogin;
 import org.keycloak.testsuite.page.AbstractPageWithInjectedUrl;
 import org.keycloak.testsuite.page.Form;
+import org.keycloak.testsuite.pages.ConsentPage;
 import org.keycloak.testsuite.util.WaitUtils;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebElement;
 
 import java.net.URL;
+import java.util.List;
 
 import static org.keycloak.testsuite.util.WaitUtils.pause;
 
@@ -44,6 +46,9 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
     @Page
     protected OIDCLogin loginPage;
 
+    @Page
+    protected ConsentPage consentPage;
+
     public void createAlbum(String name) {
         this.driver.findElement(By.id("create-album")).click();
         Form.setInputValue(this.driver.findElement(By.id("album.name")), name);
@@ -88,13 +93,53 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
         Thread.sleep(2000);
 
         this.loginPage.form().login(username, password);
+
+        // simple check if we are at the consent page, if so just click 'Yes'
+        if (this.consentPage.isCurrent()) {
+            consentPage.confirm();
+            Thread.sleep(2000);
+        }
+    }
+
+    public void loginWithScopes(String username, String password, String... scopes) throws Exception {
+        navigateTo();
+        Thread.sleep(2000);
+        if (this.driver.getCurrentUrl().startsWith(getInjectedUrl().toString())) {
+            Thread.sleep(2000);
+            logOut();
+            navigateTo();
+        }
+
+        Thread.sleep(2000);
+
+        StringBuilder scopesValue = new StringBuilder();
+
+        for (String scope : scopes) {
+            if (scopesValue.length() != 0) {
+                scopesValue.append(" ");
+            }
+            scopesValue.append(scope);
+        }
+
+        this.driver.navigate().to(this.driver.getCurrentUrl() + " " + scopesValue);
+
+        Thread.sleep(2000);
+
+        this.loginPage.form().login(username, password);
+
+        // simple check if we are at the consent page, if so just click 'Yes'
+        if (this.consentPage.isCurrent()) {
+            consentPage.confirm();
+            Thread.sleep(2000);
+        }
     }
 
     public boolean wasDenied() {
         return this.driver.findElement(By.id("output")).getText().contains("You can not access");
     }
 
-    public void viewAlbum(String name) {
+    public void viewAlbum(String name) throws InterruptedException {
+        Thread.sleep(2000);
         By id = By.id("view-" + name);
         WaitUtils.waitUntilElement(id);
         this.driver.findElements(id).forEach(WebElement::click);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java

@@ -23,20 +23,30 @@ import org.jboss.arquillian.test.api.ArquillianResource;
 import org.jboss.shrinkwrap.api.spec.WebArchive;
 import org.junit.Test;
 import org.keycloak.admin.client.resource.AuthorizationResource;
+import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ClientsResource;
+import org.keycloak.admin.client.resource.RoleResource;
+import org.keycloak.admin.client.resource.UserResource;
+import org.keycloak.admin.client.resource.UsersResource;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.representations.idm.RoleRepresentation;
+import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.representations.idm.authorization.PolicyRepresentation;
 import org.keycloak.representations.idm.authorization.ResourceRepresentation;
 import org.keycloak.representations.idm.authorization.ResourceServerRepresentation;
 import org.keycloak.testsuite.adapter.AbstractExampleAdapterTest;
 import org.keycloak.testsuite.adapter.page.PhotozClientAuthzTestApp;
+import org.keycloak.util.JsonSerialization;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertFalse;
@@ -84,7 +94,6 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         importResourceServerSettings();
     }
 
-    @Test
     public void testCreateDeleteAlbum() throws Exception {
         try {
             this.deployer.deploy(RESOURCE_SERVER_ID);
@@ -106,7 +115,6 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         }
     }
 
-    @Test
     public void testOnlyOwnerCanDeleteAlbum() throws Exception {
         try {
             this.deployer.deploy(RESOURCE_SERVER_ID);
@@ -152,7 +160,6 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         }
     }
 
-    @Test
     public void testRegularUserCanNotAccessAdminResources() throws Exception {
         try {
             this.deployer.deploy(RESOURCE_SERVER_ID);
@@ -165,7 +172,6 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         }
     }
 
-    @Test
     public void testAdminOnlyFromSpecificAddress() throws Exception {
         try {
             this.deployer.deploy(RESOURCE_SERVER_ID);
@@ -211,6 +217,23 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
                     policy.getConfig().put("applyPolicies", "[\"Any User Policy\"]");
                     getAuthorizationResource().policies().policy(policy.getId()).update(policy);
                 }
+                if ("Any User Policy".equals(policy.getName())) {
+                    ClientResource resourceServerClient = getClientResource(RESOURCE_SERVER_ID);
+                    RoleResource manageAlbumRole = resourceServerClient.roles().get("manage-albums");
+                    RoleRepresentation roleRepresentation = manageAlbumRole.toRepresentation();
+                    List<Map> roles = JsonSerialization.readValue(policy.getConfig().get("roles"), List.class);
+
+                    roles = roles.stream().filter(new Predicate<Map>() {
+                        @Override
+                        public boolean test(Map map) {
+                            return !map.get("id").equals(roleRepresentation.getId());
+                        }
+                    }).collect(Collectors.toList());
+
+                    policy.getConfig().put("roles", JsonSerialization.writeValueAsString(roles));
+
+                    getAuthorizationResource().policies().policy(policy.getId()).update(policy);
+                }
             }
 
             this.clientPage.navigateToAdminAlbum();
@@ -241,7 +264,6 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         }
     }
 
-    @Test
     public void testAdminWithoutPermissionsToDeleteScopePermission() throws Exception {
         try {
             this.deployer.deploy(RESOURCE_SERVER_ID);
@@ -305,13 +327,115 @@ public abstract class AbstractPhotozExampleAdapterTest extends AbstractExampleAd
         }
     }
 
+    public void testClientRoleRepresentingUserConsent() throws Exception {
+        try {
+            this.deployer.deploy(RESOURCE_SERVER_ID);
+            this.clientPage.login("alice", "alice");
+
+            assertFalse(this.clientPage.wasDenied());
+
+            UsersResource usersResource = realmsResouce().realm(REALM_NAME).users();
+            List<UserRepresentation> users = usersResource.search("alice", null, null, null, null, null);
+
+            assertFalse(users.isEmpty());
+
+            UserRepresentation userRepresentation = users.get(0);
+            UserResource userResource = usersResource.get(userRepresentation.getId());
+
+            ClientResource html5ClientApp = getClientResource("photoz-html5-client");
+
+            userResource.revokeConsent(html5ClientApp.toRepresentation().getClientId());
+
+            ClientResource resourceServerClient = getClientResource(RESOURCE_SERVER_ID);
+            RoleResource roleResource = resourceServerClient.roles().get("manage-albums");
+            RoleRepresentation roleRepresentation = roleResource.toRepresentation();
+
+            roleRepresentation.setScopeParamRequired(true);
+
+            roleResource.update(roleRepresentation);
+
+            this.clientPage.login("alice", "alice");
+
+            assertTrue(this.clientPage.wasDenied());
+
+            this.clientPage.loginWithScopes("alice", "alice", RESOURCE_SERVER_ID + "/manage-albums");
+
+            assertFalse(this.clientPage.wasDenied());
+        } finally {
+            this.deployer.undeploy(RESOURCE_SERVER_ID);
+        }
+    }
+
+    public void testClientRoleNotRequired() throws Exception {
+        try {
+            this.deployer.deploy(RESOURCE_SERVER_ID);
+            this.clientPage.login("alice", "alice");
+
+            assertFalse(this.clientPage.wasDenied());
+
+            UsersResource usersResource = realmsResouce().realm(REALM_NAME).users();
+            List<UserRepresentation> users = usersResource.search("alice", null, null, null, null, null);
+
+            assertFalse(users.isEmpty());
+
+            UserRepresentation userRepresentation = users.get(0);
+            UserResource userResource = usersResource.get(userRepresentation.getId());
+
+            ClientResource html5ClientApp = getClientResource("photoz-html5-client");
+
+            userResource.revokeConsent(html5ClientApp.toRepresentation().getClientId());
+
+            ClientResource resourceServerClient = getClientResource(RESOURCE_SERVER_ID);
+            RoleResource manageAlbumRole = resourceServerClient.roles().get("manage-albums");
+            RoleRepresentation roleRepresentation = manageAlbumRole.toRepresentation();
+
+            roleRepresentation.setScopeParamRequired(true);
+
+            manageAlbumRole.update(roleRepresentation);
+
+            this.clientPage.login("alice", "alice");
+
+            assertTrue(this.clientPage.wasDenied());
+
+            for (PolicyRepresentation policy : getAuthorizationResource().policies().policies()) {
+                if ("Any User Policy".equals(policy.getName())) {
+                    List<Map> roles = JsonSerialization.readValue(policy.getConfig().get("roles"), List.class);
+
+                    roles.forEach(new Consumer<Map>() {
+                        @Override
+                        public void accept(Map role) {
+                            String roleId = (String) role.get("id");
+                            if (roleId.equals(manageAlbumRole.toRepresentation().getId())) {
+                                role.put("required", false);
+                            }
+                        }
+                    });
+
+                    policy.getConfig().put("roles", JsonSerialization.writeValueAsString(roles));
+
+                    getAuthorizationResource().policies().policy(policy.getId()).update(policy);
+                }
+            }
+
+            this.clientPage.login("alice", "alice");
+
+            assertFalse(this.clientPage.wasDenied());
+        } finally {
+            this.deployer.undeploy(RESOURCE_SERVER_ID);
+        }
+    }
+
     private void importResourceServerSettings() throws FileNotFoundException {
         getAuthorizationResource().importSettings(loadJson(new FileInputStream(new File(TEST_APPS_HOME_DIR + "/photoz/photoz-restful-api-authz-service.json")), ResourceServerRepresentation.class));
     }
 
     private AuthorizationResource getAuthorizationResource() throws FileNotFoundException {
+        return getClientResource(RESOURCE_SERVER_ID).authorization();
+    }
+
+    private ClientResource getClientResource(String clientId) {
         ClientsResource clients = this.realmsResouce().realm(REALM_NAME).clients();
-        ClientRepresentation resourceServer = clients.findByClientId(RESOURCE_SERVER_ID).get(0);
+        ClientRepresentation resourceServer = clients.findByClientId(clientId).get(0);
-        return clients.get(resourceServer.getId()).authorization();
+        return clients.get(resourceServer.getId());
     }
 }