Merge pull request #998 from pedroigor/master

[KEYCLOAK-883] - Minor changes to the configuration of identity providers for clients.

forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/model/IdentityProviderBean.java

@@ -22,7 +22,9 @@
 package org.keycloak.login.freemarker.model;
 
 import org.keycloak.OAuth2Constants;
+import org.keycloak.models.ApplicationModel;
 import org.keycloak.models.ClientModel;
+import org.keycloak.models.Constants;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.services.resources.flows.Urls;
@@ -57,12 +59,19 @@ public class IdentityProviderBean {
                         ClientModel clientModel = realm.findClient(clientId);
 
                         if (clientModel != null && !clientModel.hasIdentityProvider(identityProvider.getId())) {
+                            if (ApplicationModel.class.isInstance(clientModel)) {
+                                ApplicationModel applicationModel = (ApplicationModel) clientModel;
+
+                                if (applicationModel.getName().equals(Constants.ACCOUNT_MANAGEMENT_APP)) {
+                                    addIdentityProvider(realm, baseURI, identityProvider);
+                                }
+                            }
+
                             continue;
                         }
                     }
 
-                    String loginUrl = Urls.identityProviderAuthnRequest(baseURI, identityProvider.getId(), realm.getName()).toString();
+                    addIdentityProvider(realm, baseURI, identityProvider);
-                    providers.add(new IdentityProvider(identityProvider.getId(), identityProvider.getName(), loginUrl));
                 }
             }
 
@@ -72,6 +81,11 @@ public class IdentityProviderBean {
         }
     }
 
+    private void addIdentityProvider(RealmModel realm, URI baseURI, IdentityProviderModel identityProvider) {
+        String loginUrl = Urls.identityProviderAuthnRequest(baseURI, identityProvider.getId(), realm.getName()).toString();
+        providers.add(new IdentityProvider(identityProvider.getId(), identityProvider.getName(), loginUrl));
+    }
+
     public List<IdentityProvider> getProviders() {
         return providers;
     }

model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -473,6 +473,17 @@ public class RepresentationToModel {
             applicationModel.setProtocolMappers(ids);
         }
 
+        List<String> allowedIdentityProviders = resourceRep.getAllowedIdentityProviders();
+
+        if (allowedIdentityProviders == null || allowedIdentityProviders.isEmpty()) {
+            allowedIdentityProviders = new ArrayList<String>();
+
+            for (IdentityProviderModel identityProvider : realm.getIdentityProviders()) {
+                allowedIdentityProviders.add(identityProvider.getId());
+            }
+        }
+
+        applicationModel.updateAllowedIdentityProviders(allowedIdentityProviders);
 
         return applicationModel;
     }
@@ -601,6 +612,19 @@ public class RepresentationToModel {
 
     public static OAuthClientModel createOAuthClient(OAuthClientRepresentation rep, RealmModel realm) {
         OAuthClientModel model = createOAuthClient(rep.getId(), rep.getName(), realm);
+
+        List<String> allowedIdentityProviders = rep.getAllowedIdentityProviders();
+
+        if (allowedIdentityProviders == null || allowedIdentityProviders.isEmpty()) {
+            allowedIdentityProviders = new ArrayList<String>();
+
+            for (IdentityProviderModel identityProvider : realm.getIdentityProviders()) {
+                allowedIdentityProviders.add(identityProvider.getId());
+            }
+        }
+
+        model.updateAllowedIdentityProviders(allowedIdentityProviders);
+
         updateOAuthClient(rep, model);
         return model;
     }

model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedClient.java

@@ -130,10 +130,6 @@ public class CachedClient {
     }
 
     public boolean hasIdentityProvider(String providerId) {
-        if (this.allowedIdentityProviders.isEmpty()) {
-            return true;
-        }
-
         return this.allowedIdentityProviders.contains(providerId);
     }
 

model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java

@@ -343,11 +343,6 @@ public abstract class ClientAdapter implements ClientModel {
     @Override
     public boolean hasIdentityProvider(String providerId) {
         List<String> allowedIdentityProviders = getAllowedIdentityProviders();
-
-        if (allowedIdentityProviders.isEmpty()) {
-            return true;
-        }
-
         return allowedIdentityProviders.contains(providerId);
     }
 

model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/ClientAdapter.java

@@ -341,11 +341,6 @@ public abstract class ClientAdapter<T extends MongoIdentifiableEntity> extends A
     @Override
     public boolean hasIdentityProvider(String providerId) {
         List<String> allowedIdentityProviders = getMongoEntityAsClient().getAllowedIdentityProviders();
-
-        if (allowedIdentityProviders.isEmpty()) {
-            return true;
-        }
-
         return allowedIdentityProviders.contains(providerId);
     }
 }

services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java

@@ -391,7 +391,6 @@ public class IdentityBrokerService {
         ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
 
         if (clientCode != null && clientCode.isValid(AUTHENTICATE)) {
-            validateClientPermissions(clientCode, providerId);
             ClientSessionModel clientSession = clientCode.getClientSession();
 
             if (clientSession != null) {
@@ -405,6 +404,8 @@ public class IdentityBrokerService {
                 if (clientSession.getUserSession() != null) {
                     this.event.session(clientSession.getUserSession());
                 }
+            } else {
+                validateClientPermissions(clientCode, providerId);
             }
 
             if (isDebugEnabled()) {

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

@@ -1,6 +1,7 @@
 package org.keycloak.services.resources.admin;
 
 import org.jboss.resteasy.annotations.cache.NoCache;
+import org.keycloak.models.ClientModel;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
@@ -16,6 +17,7 @@ import javax.ws.rs.GET;
 import javax.ws.rs.PUT;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.Response;
+import java.util.List;
 
 /**
  * @author Pedro Igor
@@ -42,6 +44,8 @@ public class IdentityProviderResource {
     @DELETE
     @NoCache
     public Response delete() {
+        removeClientIdentityProviders(this.realm.getApplications(), this.identityProviderModel);
+        removeClientIdentityProviders(this.realm.getApplications(), this.identityProviderModel);
         this.realm.removeIdentityProviderById(this.identityProviderModel.getId());
         return Response.noContent().build();
     }
@@ -56,4 +60,15 @@ public class IdentityProviderResource {
             return Flows.errors().exists("Identity Provider " + model.getId() + " already exists");
         }
     }
+
+    private void removeClientIdentityProviders(List<? extends ClientModel> clients, IdentityProviderModel identityProvider) {
+        for (ClientModel clientModel : clients) {
+            List<String> allowedIdentityProviders = clientModel.getAllowedIdentityProviders();
+
+            allowedIdentityProviders.remove(identityProvider.getId());
+
+            clientModel.updateAllowedIdentityProviders(allowedIdentityProviders);
+        }
+    }
+
 }

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

@@ -7,6 +7,7 @@ import org.jboss.resteasy.spi.NotFoundException;
 import org.jboss.resteasy.spi.ResteasyProviderFactory;
 import org.keycloak.broker.provider.IdentityProvider;
 import org.keycloak.broker.provider.IdentityProviderFactory;
+import org.keycloak.models.ClientModel;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
@@ -89,6 +90,10 @@ public class IdentityProvidersResource {
 
         try {
             this.realm.addIdentityProvider(RepresentationToModel.toModel(representation));
+
+            updateClientIdentityProviders(this.realm.getApplications(), representation);
+            updateClientIdentityProviders(this.realm.getOAuthClients(), representation);
+
             return Response.created(uriInfo.getAbsolutePathBuilder().path(representation.getProviderId()).build()).build();
         } catch (ModelDuplicateException e) {
             return Flows.errors().exists("Identity Provider " + representation.getId() + " already exists");
@@ -171,4 +176,14 @@ public class IdentityProvidersResource {
 
         return allProviders;
     }
+
+    private void updateClientIdentityProviders(List<? extends ClientModel> clients, IdentityProviderRepresentation identityProvider) {
+        for (ClientModel clientModel : clients) {
+            List<String> allowedIdentityProviders = clientModel.getAllowedIdentityProviders();
+
+            allowedIdentityProviders.add(identityProvider.getId());
+
+            clientModel.updateAllowedIdentityProviders(allowedIdentityProviders);
+        }
+    }
 }

testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java

@@ -24,6 +24,7 @@ import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuth2Constants;
+import org.keycloak.models.ApplicationModel;
 import org.keycloak.models.FederatedIdentityModel;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
@@ -150,6 +151,37 @@ public abstract class AbstractIdentityProviderTest {
         }
     }
 
+    @Test
+    public void testDisabledForApplication() {
+        IdentityProviderModel identityProviderModel = getIdentityProviderModel();
+        RealmModel realm = getRealm();
+        ApplicationModel applicationModel = realm.getApplicationByName("test-app");
+        List<String> allowedIdentityProviders = applicationModel.getAllowedIdentityProviders();
+
+        assertTrue(allowedIdentityProviders.contains(identityProviderModel.getId()));
+
+        allowedIdentityProviders.remove(identityProviderModel.getId());
+
+        this.driver.navigate().to("http://localhost:8081/test-app/");
+
+        assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/login"));
+
+        try {
+            this.driver.findElement(By.className(getProviderId()));
+            fail("Provider [" + getProviderId() + "] not disabled.");
+        } catch (NoSuchElementException nsee) {
+
+        }
+
+        allowedIdentityProviders.add(identityProviderModel.getId());
+
+        applicationModel.updateAllowedIdentityProviders(allowedIdentityProviders);
+
+        this.driver.navigate().to("http://localhost:8081/test-app/");
+
+        this.driver.findElement(By.className(getProviderId()));
+    }
+
     @Test
     public void testUserAlreadyExistsWhenUpdatingProfile() {
         this.driver.navigate().to("http://localhost:8081/test-app/");

testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java

@@ -45,7 +45,6 @@ import java.util.Set;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -114,25 +113,6 @@ public class ImportIdentityProviderTest extends AbstractIdentityProviderModelTes
         assertFalse(identityProviderModel.isAuthenticateByDefault());
     }
 
-    @Test
-    public void testRemoveIdentityProvider() throws Exception {
-        RealmModel realm = installTestRealm();
-        List<IdentityProviderModel> identityProviders = realm.getIdentityProviders();
-
-        assertFalse(identityProviders.isEmpty());
-
-        IdentityProviderModel identityProviderModel = identityProviders.get(0);
-        String expectedId = identityProviderModel.getId();
-
-        realm.removeIdentityProviderById(expectedId);
-
-        commit();
-
-        realm = this.realmManager.getRealm(realm.getId());
-
-        assertNull(realm.getIdentityProviderById(expectedId));
-    }
-
     private void assertIdentityProviderConfig(List<IdentityProviderModel> identityProviders) {
         assertFalse(identityProviders.isEmpty());
 

testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json

@@ -185,9 +185,20 @@
             "redirectUris": [
                 "/test-app/*"
             ],
+            "webOrigins": []
+        },
+        {
+          "name": "test-app-with-allowed-providers",
+          "enabled": true,
+          "publicClient": true,
+          "adminUrl": "http://localhost:8081/auth",
+          "baseUrl": "http://localhost:8081/auth",
+          "redirectUris": [
+            "/test-app/*"
+          ],
           "webOrigins": [],
           "allowedIdentityProviders": [
-                "model-oidc-idp"
+            "kc-oidc-idp"
           ]
         }
     ],

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

@@ -15,6 +15,7 @@
     "identityProviders" : [
         {
             "providerId" : "google",
+            "id" : "google",
             "name" : "Google",
             "enabled": true,
             "config": {