From 6e38964838f72dd02a9b3262897d1b90a2fab012 Mon Sep 17 00:00:00 2001 From: pedroigor Date: Wed, 25 Feb 2015 18:48:01 -0300 Subject: [PATCH] [KEYCLOAK-883] - Minor changes to the configuration of identity providers for clients. --- .../model/IdentityProviderBean.java | 18 +++++++++-- .../models/utils/RepresentationToModel.java | 24 ++++++++++++++ .../models/cache/entities/CachedClient.java | 4 --- .../keycloak/models/jpa/ClientAdapter.java | 5 --- .../keycloak/adapters/ClientAdapter.java | 5 --- .../resources/IdentityBrokerService.java | 3 +- .../admin/IdentityProviderResource.java | 15 +++++++++ .../admin/IdentityProvidersResource.java | 15 +++++++++ .../broker/AbstractIdentityProviderTest.java | 32 +++++++++++++++++++ .../broker/ImportIdentityProviderTest.java | 20 ------------ .../broker-test/test-realm-with-broker.json | 19 ++++++++--- .../src/test/resources/model/testrealm.json | 1 + 12 files changed, 120 insertions(+), 41 deletions(-) diff --git a/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/model/IdentityProviderBean.java b/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/model/IdentityProviderBean.java index dfa2f7159e..12a73eeb2c 100755 --- a/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/model/IdentityProviderBean.java +++ b/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(); - providers.add(new IdentityProvider(identityProvider.getId(), identityProvider.getName(), loginUrl)); + addIdentityProvider(realm, baseURI, identityProvider); } } @@ -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 getProviders() { return providers; } diff --git a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 9e152b8137..f296764114 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -473,6 +473,17 @@ public class RepresentationToModel { applicationModel.setProtocolMappers(ids); } + List allowedIdentityProviders = resourceRep.getAllowedIdentityProviders(); + + if (allowedIdentityProviders == null || allowedIdentityProviders.isEmpty()) { + allowedIdentityProviders = new ArrayList(); + + 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 allowedIdentityProviders = rep.getAllowedIdentityProviders(); + + if (allowedIdentityProviders == null || allowedIdentityProviders.isEmpty()) { + allowedIdentityProviders = new ArrayList(); + + for (IdentityProviderModel identityProvider : realm.getIdentityProviders()) { + allowedIdentityProviders.add(identityProvider.getId()); + } + } + + model.updateAllowedIdentityProviders(allowedIdentityProviders); + updateOAuthClient(rep, model); return model; } diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedClient.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedClient.java index 08fa1e7064..1029713ba6 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedClient.java +++ b/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); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java index d4616fd526..c437014cbc 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java +++ b/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 allowedIdentityProviders = getAllowedIdentityProviders(); - - if (allowedIdentityProviders.isEmpty()) { - return true; - } - return allowedIdentityProviders.contains(providerId); } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/ClientAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/ClientAdapter.java index 8d87dc0b33..3c472821f6 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/ClientAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/ClientAdapter.java @@ -341,11 +341,6 @@ public abstract class ClientAdapter extends A @Override public boolean hasIdentityProvider(String providerId) { List allowedIdentityProviders = getMongoEntityAsClient().getAllowedIdentityProviders(); - - if (allowedIdentityProviders.isEmpty()) { - return true; - } - return allowedIdentityProviders.contains(providerId); } } diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index e0a2797655..5de4015ddb 100644 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/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()) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java index 1e61b3e613..e8b184edce 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProviderResource.java +++ b/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 clients, IdentityProviderModel identityProvider) { + for (ClientModel clientModel : clients) { + List allowedIdentityProviders = clientModel.getAllowedIdentityProviders(); + + allowedIdentityProviders.remove(identityProvider.getId()); + + clientModel.updateAllowedIdentityProviders(allowedIdentityProviders); + } + } + } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java index a8ea3a4c15..d44e464c44 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/IdentityProvidersResource.java +++ b/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 clients, IdentityProviderRepresentation identityProvider) { + for (ClientModel clientModel : clients) { + List allowedIdentityProviders = clientModel.getAllowedIdentityProviders(); + + allowedIdentityProviders.add(identityProvider.getId()); + + clientModel.updateAllowedIdentityProviders(allowedIdentityProviders); + } + } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java index 473c375e34..6921ec3132 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java +++ b/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 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/"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java index 36d2bda835..d654d1e5d9 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java +++ b/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 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 identityProviders) { assertFalse(identityProviders.isEmpty()); diff --git a/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json b/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json index bc62fa85ee..322cebe423 100755 --- a/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json +++ b/testsuite/integration/src/test/resources/broker-test/test-realm-with-broker.json @@ -185,10 +185,21 @@ "redirectUris": [ "/test-app/*" ], - "webOrigins": [], - "allowedIdentityProviders" : [ - "model-oidc-idp" - ] + "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": [ + "kc-oidc-idp" + ] } ], "oauthClients" : [ diff --git a/testsuite/integration/src/test/resources/model/testrealm.json b/testsuite/integration/src/test/resources/model/testrealm.json index be9279a576..8d36a576fc 100755 --- a/testsuite/integration/src/test/resources/model/testrealm.json +++ b/testsuite/integration/src/test/resources/model/testrealm.json @@ -15,6 +15,7 @@ "identityProviders" : [ { "providerId" : "google", + "id" : "google", "name" : "Google", "enabled": true, "config": {