Merge pull request #2675 from mposolda/master

Client template fixes

core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java

@@ -264,13 +264,22 @@ public class RealmRepresentation {
         return scopeMappings;
     }
 
-    public ScopeMappingRepresentation scopeMapping(String username) {
+    public ScopeMappingRepresentation clientScopeMapping(String clientName) {
         ScopeMappingRepresentation mapping = new ScopeMappingRepresentation();
-        mapping.setClient(username);
+        mapping.setClient(clientName);
         if (scopeMappings == null) scopeMappings = new ArrayList<ScopeMappingRepresentation>();
         scopeMappings.add(mapping);
         return mapping;
     }
+
+    public ScopeMappingRepresentation clientTemplateScopeMapping(String clientTemplateName) {
+        ScopeMappingRepresentation mapping = new ScopeMappingRepresentation();
+        mapping.setClientTemplate(clientTemplateName);
+        if (scopeMappings == null) scopeMappings = new ArrayList<ScopeMappingRepresentation>();
+        scopeMappings.add(mapping);
+        return mapping;
+    }
+
     @Deprecated
     public Set<String> getRequiredCredentials() {
         return requiredCredentials;

core/src/main/java/org/keycloak/representations/idm/ScopeMappingRepresentation.java

@@ -27,6 +27,7 @@ import java.util.Set;
 public class ScopeMappingRepresentation {
     protected String self; // link
     protected String client;
+    protected String clientTemplate;
     protected Set<String> roles;
 
     public String getSelf() {
@@ -45,6 +46,14 @@ public class ScopeMappingRepresentation {
         this.client = client;
     }
 
+    public String getClientTemplate() {
+        return clientTemplate;
+    }
+
+    public void setClientTemplate(String clientTemplate) {
+        this.clientTemplate = clientTemplate;
+    }
+
     public Set<String> getRoles() {
         return roles;
     }

server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java

@@ -632,5 +632,15 @@ public final class KeycloakModelUtils {
         return false;
     }
 
+    public static ClientTemplateModel getClientTemplateByName(RealmModel realm, String templateName) {
+        for (ClientTemplateModel clientTemplate : realm.getClientTemplates()) {
+            if (templateName.equals(clientTemplate.getName())) {
+                return clientTemplate;
+            }
+        }
+
+        return null;
+    }
+
 
 }

server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -42,6 +42,7 @@ import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RequiredActionProviderModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.ScopeContainerModel;
 import org.keycloak.models.UserConsentModel;
 import org.keycloak.models.UserCredentialModel;
 import org.keycloak.models.UserCredentialValueModel;
@@ -250,16 +251,14 @@ public class RepresentationToModel {
 
         if (rep.getScopeMappings() != null) {
             for (ScopeMappingRepresentation scope : rep.getScopeMappings()) {
-                ClientModel client = newRealm.getClientByClientId(scope.getClient());
+                ScopeContainerModel scopeContainer = getScopeContainerHavingScope(newRealm, scope);
-                if (client == null) {
+
-                    throw new RuntimeException("Unknown client specification in realm scope mappings");
-                }
                 for (String roleString : scope.getRoles()) {
                     RoleModel role = newRealm.getRole(roleString.trim());
                     if (role == null) {
                         role = newRealm.addRole(roleString.trim());
                     }
-                    client.addScopeMapping(role);
+                    scopeContainer.addScopeMapping(role);
                 }
 
             }
@@ -1205,20 +1204,36 @@ public class RepresentationToModel {
 
     public static void createClientScopeMappings(RealmModel realm, ClientModel clientModel, List<ScopeMappingRepresentation> mappings) {
         for (ScopeMappingRepresentation mapping : mappings) {
-            ClientModel client = realm.getClientByClientId(mapping.getClient());
+            ScopeContainerModel scopeContainer = getScopeContainerHavingScope(realm, mapping);
-            if (client == null) {
+
-                throw new RuntimeException("Unknown client specified in client scope mappings");
-            }
             for (String roleString : mapping.getRoles()) {
                 RoleModel role = clientModel.getRole(roleString.trim());
                 if (role == null) {
                     role = clientModel.addRole(roleString.trim());
                 }
-                client.addScopeMapping(role);
+                scopeContainer.addScopeMapping(role);
             }
         }
     }
 
+    private static ScopeContainerModel getScopeContainerHavingScope(RealmModel realm, ScopeMappingRepresentation scope) {
+        if (scope.getClient() != null) {
+            ClientModel client = realm.getClientByClientId(scope.getClient());
+            if (client == null) {
+                throw new RuntimeException("Unknown client specification in scope mappings: " + scope.getClient());
+            }
+            return client;
+        } else if (scope.getClientTemplate() != null) {
+            ClientTemplateModel clientTemplate = KeycloakModelUtils.getClientTemplateByName(realm, scope.getClientTemplate());
+            if (clientTemplate == null) {
+                throw new RuntimeException("Unknown clientTemplate specification in scope mappings: " + scope.getClientTemplate());
+            }
+            return clientTemplate;
+        } else {
+            throw new RuntimeException("Either client or clientTemplate needs to be specified in scope mappings");
+        }
+    }
+
     // Users
 
     public static UserModel createUser(KeycloakSession session, RealmModel newRealm, UserRepresentation userRep) {

services/src/main/java/org/keycloak/exportimport/util/ExportUtils.java

@@ -88,13 +88,14 @@ public class ExportUtils {
         List<ClientModel> allClients = new ArrayList<>(clients);
         Map<String, List<ScopeMappingRepresentation>> clientScopeReps = new HashMap<>();
 
+        // Scopes of clients
         for (ClientModel client : allClients) {
             Set<RoleModel> clientScopes = client.getScopeMappings();
             ScopeMappingRepresentation scopeMappingRep = null;
             for (RoleModel scope : clientScopes) {
                 if (scope.getContainer() instanceof RealmModel) {
                     if (scopeMappingRep == null) {
-                        scopeMappingRep = rep.scopeMapping(client.getClientId());
+                        scopeMappingRep = rep.clientScopeMapping(client.getClientId());
                     }
                     scopeMappingRep.role(scope.getName());
                 } else {
@@ -108,7 +109,7 @@ public class ExportUtils {
 
                     ScopeMappingRepresentation currentClientScope = null;
                     for (ScopeMappingRepresentation scopeMapping : currentAppScopes) {
-                        if (scopeMapping.getClient().equals(client.getClientId())) {
+                        if (client.getClientId().equals(scopeMapping.getClient())) {
                             currentClientScope = scopeMapping;
                             break;
                         }
@@ -123,6 +124,42 @@ public class ExportUtils {
             }
         }
 
+        // Scopes of client templates
+        for (ClientTemplateModel clientTemplate : realm.getClientTemplates()) {
+            Set<RoleModel> clientScopes = clientTemplate.getScopeMappings();
+            ScopeMappingRepresentation scopeMappingRep = null;
+            for (RoleModel scope : clientScopes) {
+                if (scope.getContainer() instanceof RealmModel) {
+                    if (scopeMappingRep == null) {
+                        scopeMappingRep = rep.clientTemplateScopeMapping(clientTemplate.getName());
+                    }
+                    scopeMappingRep.role(scope.getName());
+                } else {
+                    ClientModel app = (ClientModel)scope.getContainer();
+                    String appName = app.getClientId();
+                    List<ScopeMappingRepresentation> currentAppScopes = clientScopeReps.get(appName);
+                    if (currentAppScopes == null) {
+                        currentAppScopes = new ArrayList<>();
+                        clientScopeReps.put(appName, currentAppScopes);
+                    }
+
+                    ScopeMappingRepresentation currentClientTemplateScope = null;
+                    for (ScopeMappingRepresentation scopeMapping : currentAppScopes) {
+                        if (clientTemplate.getName().equals(scopeMapping.getClientTemplate())) {
+                            currentClientTemplateScope = scopeMapping;
+                            break;
+                        }
+                    }
+                    if (currentClientTemplateScope == null) {
+                        currentClientTemplateScope = new ScopeMappingRepresentation();
+                        currentClientTemplateScope.setClientTemplate(clientTemplate.getName());
+                        currentAppScopes.add(currentClientTemplateScope);
+                    }
+                    currentClientTemplateScope.role(scope.getName());
+                }
+            }
+        }
+
         if (clientScopeReps.size() > 0) {
             rep.setClientScopeMappings(clientScopeReps);
         }

services/src/main/java/org/keycloak/services/resources/admin/ClientTemplateResource.java

@@ -22,6 +22,7 @@ import org.keycloak.events.admin.OperationType;
 import org.keycloak.models.ClientTemplateModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
+import org.keycloak.models.ModelException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.models.utils.RepresentationToModel;
@@ -133,11 +134,16 @@ public class ClientTemplateResource {
      */
     @DELETE
     @NoCache
-    public void deleteClientTemplate() {
+    public Response deleteClientTemplate() {
         auth.requireManage();
 
-        realm.removeClientTemplate(template.getId());
+        try {
-        adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).success();
+            realm.removeClientTemplate(template.getId());
+            adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).success();
+            return Response.noContent().build();
+        } catch (ModelException me) {
+            return ErrorResponse.error(me.getMessage(), Response.Status.BAD_REQUEST);
+        }
     }
 
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/endpoint/client/ClientTemplateTest.java

@@ -23,6 +23,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import javax.ws.rs.BadRequestException;
 import javax.ws.rs.NotFoundException;
 import javax.ws.rs.core.Response;
 
@@ -35,6 +36,7 @@ import org.keycloak.models.AccountRoles;
 import org.keycloak.models.Constants;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.saml.SamlProtocol;
+import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.ClientTemplateRepresentation;
 import org.keycloak.representations.idm.ErrorRepresentation;
 import org.keycloak.representations.idm.MappingsRepresentation;
@@ -270,6 +272,40 @@ public class ClientTemplateTest extends AbstractClientTest {
     }
 
 
+    // KEYCLOAK-2844
+    @Test
+    public void testRemoveTemplateInUse() {
+        // Add client template
+        ClientTemplateRepresentation templateRep = new ClientTemplateRepresentation();
+        templateRep.setName("foo-template");
+        templateRep.setFullScopeAllowed(false);
+        String templateId = createTemplate(templateRep);
+
+        // Add client with the clientTemplate
+        ClientRepresentation clientRep = new ClientRepresentation();
+        clientRep.setClientId("bar-client");
+        clientRep.setName("bar-client");
+        clientRep.setRootUrl("foo");
+        clientRep.setProtocol("openid-connect");
+        clientRep.setClientTemplate("foo-template");
+        String clientDbId = createClient(clientRep);
+
+        // Can't remove clientTemplate
+        try {
+            clientTemplates().get(templateId).remove();
+        } catch (BadRequestException bre) {
+            ErrorRepresentation error = bre.getResponse().readEntity(ErrorRepresentation.class);
+            Assert.assertEquals("Cannot remove client template, it is currently in use", error.getErrorMessage());
+        }
+
+        // Remove client
+        testRealmResource().clients().get(clientDbId).remove();
+
+        // Can remove clientTemplate now
+        clientTemplates().get(templateId).remove();
+    }
+
+
     private ClientTemplatesResource clientTemplates() {
         return testRealmResource().clientTemplates();
     }

testsuite/integration/src/test/java/org/keycloak/testsuite/model/CompositeRolesModelTest.java

@@ -58,7 +58,7 @@ public class CompositeRolesModelTest extends AbstractModelTest {
         RealmRepresentation rep = AbstractModelTest.loadJson("model/testrealm-noclient-id.json");
         rep.setId("TestNoClientID");
         expectedException.expect(RuntimeException.class);
-        expectedException.expectMessage("Unknown client specified in client scope mappings");
+        expectedException.expectMessage("Unknown client specification in scope mappings: some-client");
         manager.importRealm(rep);
 
     }

testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java

@@ -26,6 +26,7 @@ import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapper;
 import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapperFactory;
 import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientModel;
+import org.keycloak.models.ClientTemplateModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.FederatedIdentityModel;
 import org.keycloak.models.IdentityProviderModel;
@@ -321,13 +322,32 @@ public class ImportTest extends AbstractModelTest {
         Assert.assertEquals(1, otherApp.getProtocolMappers().size());
         Assert.assertNull(otherApp.getProtocolMapperByName(OIDCLoginProtocol.LOGIN_PROTOCOL, "username"));
         ProtocolMapperModel gssCredentialMapper = otherApp.getProtocolMapperByName(OIDCLoginProtocol.LOGIN_PROTOCOL, KerberosConstants.GSS_DELEGATION_CREDENTIAL_DISPLAY_NAME);
-        Assert.assertEquals(KerberosConstants.GSS_DELEGATION_CREDENTIAL_DISPLAY_NAME, gssCredentialMapper.getName());
+        assertGssProtocolMapper(gssCredentialMapper);
-        Assert.assertEquals( OIDCLoginProtocol.LOGIN_PROTOCOL, gssCredentialMapper.getProtocol());
+
-        Assert.assertEquals(UserSessionNoteMapper.PROVIDER_ID, gssCredentialMapper.getProtocolMapper());
+        // Test clientTemplates
-        String includeInAccessToken = gssCredentialMapper.getConfig().get(OIDCAttributeMapperHelper.INCLUDE_IN_ACCESS_TOKEN);
+        List<ClientTemplateModel> clientTemplates = realm.getClientTemplates();
-        String includeInIdToken = gssCredentialMapper.getConfig().get(OIDCAttributeMapperHelper.INCLUDE_IN_ID_TOKEN);
+        Assert.assertEquals(1, clientTemplates.size());
-        Assert.assertTrue(includeInAccessToken.equalsIgnoreCase("true"));
+        ClientTemplateModel clientTemplate = clientTemplates.get(0);
-        Assert.assertTrue(includeInIdToken == null || Boolean.parseBoolean(includeInIdToken) == false);
+        Assert.assertEquals("foo-template", clientTemplate.getName());
+        Assert.assertEquals("foo-template-desc", clientTemplate.getDescription());
+        Assert.assertEquals(OIDCLoginProtocol.LOGIN_PROTOCOL, clientTemplate.getProtocol());
+        Assert.assertEquals(1, clientTemplate.getProtocolMappers().size());
+        ProtocolMapperModel templateGssCredentialMapper = clientTemplate.getProtocolMapperByName(OIDCLoginProtocol.LOGIN_PROTOCOL, KerberosConstants.GSS_DELEGATION_CREDENTIAL_DISPLAY_NAME);
+        assertGssProtocolMapper(templateGssCredentialMapper);
+
+        // Test client template scopes
+        Set<RoleModel> allClientTemplateScopes = clientTemplate.getScopeMappings();
+        Assert.assertEquals(3, allClientTemplateScopes.size());
+        Assert.assertTrue(allClientTemplateScopes.contains(realm.getRole("admin")));
+        Assert.assertTrue(allClientTemplateScopes.contains(application.getRole("app-user")));
+        Assert.assertTrue(allClientTemplateScopes.contains(application.getRole("app-admin")));
+
+        Set<RoleModel> clientTemplateRealmScopes = clientTemplate.getRealmScopeMappings();
+        Assert.assertTrue(clientTemplateRealmScopes.contains(realm.getRole("admin")));
+
+        Set<RoleModel> clientTemplateAppScopes = KeycloakModelUtils.getClientScopeMappings(application, clientTemplate);//application.getClientScopeMappings(oauthClient);
+        Assert.assertTrue(clientTemplateAppScopes.contains(application.getRole("app-user")));
+        Assert.assertTrue(clientTemplateAppScopes.contains(application.getRole("app-admin")));
 
         // Test user consents
         admin =  session.users().getUserByUsername("admin", realm);
@@ -380,4 +400,14 @@ public class ImportTest extends AbstractModelTest {
         Assert.assertEquals(expectedType, requiredCreds.get(0).getType());
     }
 
+    private static void assertGssProtocolMapper(ProtocolMapperModel gssCredentialMapper) {
+        Assert.assertEquals(KerberosConstants.GSS_DELEGATION_CREDENTIAL_DISPLAY_NAME, gssCredentialMapper.getName());
+        Assert.assertEquals( OIDCLoginProtocol.LOGIN_PROTOCOL, gssCredentialMapper.getProtocol());
+        Assert.assertEquals(UserSessionNoteMapper.PROVIDER_ID, gssCredentialMapper.getProtocolMapper());
+        String includeInAccessToken = gssCredentialMapper.getConfig().get(OIDCAttributeMapperHelper.INCLUDE_IN_ACCESS_TOKEN);
+        String includeInIdToken = gssCredentialMapper.getConfig().get(OIDCAttributeMapperHelper.INCLUDE_IN_ID_TOKEN);
+        Assert.assertTrue(includeInAccessToken.equalsIgnoreCase("true"));
+        Assert.assertTrue(includeInIdToken == null || Boolean.parseBoolean(includeInIdToken) == false);
+    }
+
 }

testsuite/integration/src/test/resources/model/testrealm.json

@@ -195,6 +195,28 @@
             "secret": "clientpassword"
         }
     ],
+    "clientTemplates" : [
+        {
+            "name" : "foo-template",
+            "description" : "foo-template-desc",
+            "protocol" : "openid-connect",
+            "protocolMappers" : [
+                {
+                    "name" : "gss delegation credential",
+                    "protocol" : "openid-connect",
+                    "protocolMapper" : "oidc-usersessionmodel-note-mapper",
+                    "consentRequired" : true,
+                    "consentText" : "gss delegation credential",
+                    "config" : {
+                        "user.session.note" : "gss_delegation_credential",
+                        "access.token.claim" : "true",
+                        "claim.name" : "gss_delegation_credential",
+                        "Claim JSON Type" : "String"
+                    }
+                }
+            ]
+        }
+    ],
     "roles" : {
         "realm" : [
             {
@@ -226,6 +248,10 @@
         {
             "client": "oauthclient",
             "roles": ["admin"]
+        },
+        {
+            "clientTemplate": "foo-template",
+            "roles": ["admin"]
         }
     ],
     "applicationScopeMappings": {
@@ -233,6 +259,10 @@
             {
                 "client": "oauthclient",
                 "roles": ["app-user"]
+            },
+            {
+                "clientTemplate": "foo-template",
+                "roles": ["app-user", "app-admin" ]
             }
         ]