From aeb27ff047c12589afd8236c89ab8909804f3279 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 17 Mar 2015 22:16:19 +0100 Subject: [PATCH] KEYCLOAK-1108 Remove option for enable/disable login per application --- .../en/en-US/modules/identity-broker.xml | 65 ++++++++----------- .../resources/js/controllers/applications.js | 56 ++++++++-------- .../application-identity-provider.html | 4 -- .../model/IdentityProviderBean.java | 18 ----- .../java/org/keycloak/models/ClientModel.java | 3 +- .../models/utils/RepresentationToModel.java | 34 ++++------ .../models/file/adapter/ClientAdapter.java | 15 +---- .../keycloak/models/cache/ClientAdapter.java | 10 +-- .../keycloak/models/jpa/ClientAdapter.java | 13 +--- .../keycloak/adapters/ClientAdapter.java | 15 +---- .../resources/IdentityBrokerService.java | 27 ++------ .../admin/IdentityProviderResource.java | 65 ++++++++++++++++--- .../admin/IdentityProvidersResource.java | 16 ----- .../broker/AbstractIdentityProviderTest.java | 55 +++++++--------- .../broker/ImportIdentityProviderTest.java | 2 +- 15 files changed, 155 insertions(+), 243 deletions(-) diff --git a/docbook/reference/en/en-US/modules/identity-broker.xml b/docbook/reference/en/en-US/modules/identity-broker.xml index 673f00a0d5..0ab53f0c05 100755 --- a/docbook/reference/en/en-US/modules/identity-broker.xml +++ b/docbook/reference/en/en-US/modules/identity-broker.xml @@ -954,6 +954,33 @@ Authorization: Bearer {keycloak_access_token}]]> In this case, given that you are accessing an protected service in Keycloak, you need to send the access token issued by Keycloak during the user authentication. + + By default, the Keycloak access token issued for the application can't be automatically used for retrieve thirdparty token. You will + need to enable this in admin console first: + + + + Click 'Applications' on the left side menu. + + + + + Select an application from the list. + + + + + Click the 'Identity Provider' tab. + + + + + From this page you can configure if an application is allowed to retrieve tokens from an specific identity provider. For that, + just click on the Can Retrieve Token button. + + + + If your application is not at the same origin as the authentication server, make sure you have properly configured CORS. @@ -961,44 +988,6 @@ Authorization: Bearer {keycloak_access_token}]]> -
- Configuring Identity Providers for Applications - - By default, all identity providers enabled for a particular realm are also available to all its applications. - However, you can also specify which identity providers should be available when - authenticating users for a particular application. - - - For that, please follow these steps: - - - - - Click 'Applications' on the left side menu. - - - - - Select an application from the list. - - - - - Click the 'Identity Provider' tab. - - - - - Now you should be able to enable or disabled identity providers to the application by clicking on the Enable/Disable button. - - - - - From this page you can also configure if an application is allowed to retrieve tokens from an specific identity provider. For that, - just click on the Can Retrieve Token button. - -
-
Automatically Select and Identity Provider diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/applications.js b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/applications.js index 5552bfac28..2334c9bdcd 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/applications.js +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/applications.js @@ -56,35 +56,34 @@ module.controller('ApplicationIdentityProviderCtrl', function($scope, $location, if ($scope.application.identityProviders) { length = $scope.application.identityProviders.length; - } else { - $scope.application.identityProviders = new Array(realm.identityProviders.length); - } - for (j = length; j < realm.identityProviders.length; j++) { - $scope.application.identityProviders[j] = {}; + for (i = 0; i < $scope.application.identityProviders.length; i++) { + var applicationProvider = $scope.application.identityProviders[i]; + if (applicationProvider.retrieveToken) { + applicationProvider.retrieveToken = applicationProvider.retrieveToken.toString(); + } + } + + } else { + $scope.application.identityProviders = []; } $scope.identityProviders = []; + var providersMissingInApp = []; for (j = 0; j < realm.identityProviders.length; j++) { var identityProvider = realm.identityProviders[j]; - var match = false; - var applicationProvider; + var applicationProvider = null; for (i = 0; i < $scope.application.identityProviders.length; i++) { applicationProvider = $scope.application.identityProviders[i]; if (applicationProvider) { - if (applicationProvider.retrieveToken) { - applicationProvider.retrieveToken = applicationProvider.retrieveToken.toString(); - } else { - applicationProvider.retrieveToken = false.toString(); - } if (applicationProvider.id == identityProvider.id) { $scope.identityProviders[i] = {}; $scope.identityProviders[i].identityProvider = identityProvider; - $scope.identityProviders[i].retrieveToken = applicationProvider.retrieveToken.toString(); + $scope.identityProviders[i].retrieveToken = applicationProvider.retrieveToken; break; } @@ -93,30 +92,27 @@ module.controller('ApplicationIdentityProviderCtrl', function($scope, $location, } if (applicationProvider == null) { - var length = $scope.identityProviders.length + $scope.application.identityProviders.length; - - $scope.identityProviders[length] = {}; - $scope.identityProviders[length].identityProvider = identityProvider; - $scope.identityProviders[length].retrieveToken = false.toString(); + providersMissingInApp.push(identityProvider); } } - $scope.identityProviders = $scope.identityProviders.filter(function(n){ return n != undefined }); + for (j = 0; j < providersMissingInApp.length; j++) { + var identityProvider = providersMissingInApp[j]; + + var currentProvider = {}; + currentProvider.identityProvider = identityProvider; + currentProvider.retrieveToken = "false"; + $scope.identityProviders.push(currentProvider); + + var currentAppProvider = {}; + currentAppProvider.id = identityProvider.id; + currentAppProvider.retrieveToken = "false"; + $scope.application.identityProviders.push(currentAppProvider); + } var oldCopy = angular.copy($scope.application); $scope.save = function() { - var selectedProviders = []; - - for (i = 0; i < $scope.application.identityProviders.length; i++) { - var appProvider = $scope.application.identityProviders[i]; - - if (appProvider.id != null && appProvider.id != false) { - selectedProviders[selectedProviders.length] = appProvider; - } - } - - $scope.application.identityProviders = selectedProviders; Application.update({ realm : realm.realm, diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/application-identity-provider.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/application-identity-provider.html index aeb3b8d961..f4473bf8fa 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/application-identity-provider.html +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/application-identity-provider.html @@ -11,10 +11,6 @@
{{identityProvider.identityProvider.name}} - -
- -
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 12a73eeb2c..edb1f0756b 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 @@ -53,24 +53,6 @@ public class IdentityProviderBean { for (IdentityProviderModel identityProvider : identityProviders) { if (identityProvider.isEnabled()) { - String clientId = uriInfo.getQueryParameters().getFirst(OAuth2Constants.CLIENT_ID); - - if (clientId != null) { - 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; - } - } - addIdentityProvider(realm, baseURI, identityProvider); } } diff --git a/model/api/src/main/java/org/keycloak/models/ClientModel.java b/model/api/src/main/java/org/keycloak/models/ClientModel.java index 5f66b7d4ae..39e4d17350 100755 --- a/model/api/src/main/java/org/keycloak/models/ClientModel.java +++ b/model/api/src/main/java/org/keycloak/models/ClientModel.java @@ -98,9 +98,8 @@ public interface ClientModel { void setNotBefore(int notBefore); - void updateAllowedIdentityProviders(List identityProviders); + void updateIdentityProviders(List identityProviders); List getIdentityProviders(); - boolean hasIdentityProvider(String providerId); boolean isAllowedRetrieveTokenFromIdentityProvider(String providerId); Set getProtocolMappers(); 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 c2090d3ce9..afdd6aa051 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 @@ -489,7 +489,7 @@ public class RepresentationToModel { } } - applicationModel.updateAllowedIdentityProviders(toModel(resourceRep.getIdentityProviders(), realm)); + applicationModel.updateIdentityProviders(toModel(resourceRep.getIdentityProviders(), realm)); return applicationModel; } @@ -538,7 +538,7 @@ public class RepresentationToModel { } } - updateClientIdentityProvides(rep.getIdentityProviders(), resource); + updateClientIdentityProviders(rep.getIdentityProviders(), resource); } public static void setClaims(ClientModel model, ClaimRepresentation rep) { @@ -613,7 +613,7 @@ public class RepresentationToModel { public static OAuthClientModel createOAuthClient(OAuthClientRepresentation rep, RealmModel realm) { OAuthClientModel model = createOAuthClient(rep.getId(), rep.getName(), realm); - model.updateAllowedIdentityProviders(toModel(rep.getIdentityProviders(), realm)); + model.updateIdentityProviders(toModel(rep.getIdentityProviders(), realm)); updateOAuthClient(rep, model); return model; @@ -653,7 +653,7 @@ public class RepresentationToModel { } } - updateClientIdentityProvides(rep.getIdentityProviders(), model); + updateClientIdentityProviders(rep.getIdentityProviders(), model); if (rep.getProtocolMappers() != null) { // first, remove all default/built in mappers @@ -818,35 +818,25 @@ public class RepresentationToModel { } private static List toModel(List repIdentityProviders, RealmModel realm) { - List allowedIdentityProviders = new ArrayList(); + List result = new ArrayList(); - if (repIdentityProviders == null || repIdentityProviders.isEmpty()) { - allowedIdentityProviders = new ArrayList(); - - for (IdentityProviderModel identityProvider : realm.getIdentityProviders()) { - ClientIdentityProviderMappingModel identityProviderMapping = new ClientIdentityProviderMappingModel(); - - identityProviderMapping.setIdentityProvider(identityProvider.getId()); - - allowedIdentityProviders.add(identityProviderMapping); - } - } else { + if (repIdentityProviders != null) { for (ClientIdentityProviderMappingRepresentation rep : repIdentityProviders) { ClientIdentityProviderMappingModel identityProviderMapping = new ClientIdentityProviderMappingModel(); identityProviderMapping.setIdentityProvider(rep.getId()); identityProviderMapping.setRetrieveToken(rep.isRetrieveToken()); - allowedIdentityProviders.add(identityProviderMapping); + result.add(identityProviderMapping); } } - return allowedIdentityProviders; + return result; } - private static void updateClientIdentityProvides(List identityProviders, ClientModel resource) { + private static void updateClientIdentityProviders(List identityProviders, ClientModel resource) { if (identityProviders != null) { - List allowedIdentityProviders = new ArrayList(); + List result = new ArrayList(); for (ClientIdentityProviderMappingRepresentation mappingRepresentation : identityProviders) { ClientIdentityProviderMappingModel identityProviderMapping = new ClientIdentityProviderMappingModel(); @@ -854,10 +844,10 @@ public class RepresentationToModel { identityProviderMapping.setIdentityProvider(mappingRepresentation.getId()); identityProviderMapping.setRetrieveToken(mappingRepresentation.isRetrieveToken()); - allowedIdentityProviders.add(identityProviderMapping); + result.add(identityProviderMapping); } - resource.updateAllowedIdentityProviders(allowedIdentityProviders); + resource.updateIdentityProviders(result); } } } diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/ClientAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/ClientAdapter.java index d5eda8e5f7..c7ef8ad7cf 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/ClientAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/ClientAdapter.java @@ -386,7 +386,7 @@ public abstract class ClientAdapter implements ClientModel { } @Override - public void updateAllowedIdentityProviders(List identityProviders) { + public void updateIdentityProviders(List identityProviders) { List stored = new ArrayList(); for (ClientIdentityProviderMappingModel model : identityProviders) { @@ -416,19 +416,6 @@ public abstract class ClientAdapter implements ClientModel { return models; } - @Override - public boolean hasIdentityProvider(String providerId) { - for (ClientIdentityProviderMappingEntity identityProviderMappingModel : clientEntity.getIdentityProviders()) { - String identityProvider = identityProviderMappingModel.getId(); - - if (identityProvider.equals(providerId)) { - return true; - } - } - - return false; - } - @Override public boolean isAllowedRetrieveTokenFromIdentityProvider(String providerId) { for (ClientIdentityProviderMappingEntity identityProviderMappingModel : clientEntity.getIdentityProviders()) { diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/ClientAdapter.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/ClientAdapter.java index c15059e0e0..202be7b4ba 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/ClientAdapter.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/ClientAdapter.java @@ -264,9 +264,9 @@ public abstract class ClientAdapter implements ClientModel { } @Override - public void updateAllowedIdentityProviders(List identityProviders) { + public void updateIdentityProviders(List identityProviders) { getDelegateForUpdate(); - updatedClient.updateAllowedIdentityProviders(identityProviders); + updatedClient.updateIdentityProviders(identityProviders); } @Override @@ -275,12 +275,6 @@ public abstract class ClientAdapter implements ClientModel { return cachedClient.getIdentityProviders(); } - @Override - public boolean hasIdentityProvider(String providerId) { - if (updatedClient != null) return updatedClient.hasIdentityProvider(providerId); - return cachedClient.hasIdentityProvider(providerId); - } - @Override public boolean isAllowedRetrieveTokenFromIdentityProvider(String providerId) { if (updatedClient != null) return updatedClient.isAllowedRetrieveTokenFromIdentityProvider(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 52fe96551b..1d2cb05c07 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 @@ -307,7 +307,7 @@ public abstract class ClientAdapter implements ClientModel { } @Override - public void updateAllowedIdentityProviders(List identityProviders) { + public void updateIdentityProviders(List identityProviders) { Collection entities = entity.getIdentityProviders(); Set already = new HashSet(); List remove = new ArrayList(); @@ -377,17 +377,6 @@ public abstract class ClientAdapter implements ClientModel { return models; } - @Override - public boolean hasIdentityProvider(String providerId) { - for (ClientIdentityProviderMappingModel model : getIdentityProviders()) { - if (model.getIdentityProvider().equals(providerId)) { - return true; - } - } - - return false; - } - @Override public boolean isAllowedRetrieveTokenFromIdentityProvider(String providerId) { for (ClientIdentityProviderMappingModel model : getIdentityProviders()) { 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 f52e9ef39a..ea97d65cf3 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 @@ -411,7 +411,7 @@ public abstract class ClientAdapter extends A @Override - public void updateAllowedIdentityProviders(List identityProviders) { + public void updateIdentityProviders(List identityProviders) { List stored = new ArrayList(); for (ClientIdentityProviderMappingModel model : identityProviders) { @@ -442,19 +442,6 @@ public abstract class ClientAdapter extends A return models; } - @Override - public boolean hasIdentityProvider(String providerId) { - for (ClientIdentityProviderMappingEntity identityProviderMappingModel : getMongoEntityAsClient().getIdentityProviders()) { - String identityProvider = identityProviderMappingModel.getId(); - - if (identityProvider.equals(providerId)) { - return true; - } - } - - return false; - } - @Override public boolean isAllowedRetrieveTokenFromIdentityProvider(String providerId) { for (ClientIdentityProviderMappingEntity identityProviderMappingModel : getMongoEntityAsClient().getIdentityProviders()) { 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 4666ff4afa..2b3402bd2a 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -182,10 +182,6 @@ public class IdentityBrokerService { return badRequest("Invalid client."); } - if (!clientModel.hasIdentityProvider(providerId)) { - return corsResponse(badRequest("Client [" + audience + "] not authorized."), clientModel); - } - if (!clientModel.isAllowedRetrieveTokenFromIdentityProvider(providerId)) { return corsResponse(badRequest("Client [" + audience + "] not authorized to retrieve tokens from identity provider [" + providerId + "]."), clientModel); } @@ -399,16 +395,16 @@ public class IdentityBrokerService { if (clientSession != null) { ClientModel client = clientSession.getClient(); - if (client != null) { - LOGGER.debugf("Got authorization code from client [%s].", client.getClientId()); - this.event.client(client); + if (client == null) { + throw new IdentityBrokerException("Invalid client"); } + LOGGER.debugf("Got authorization code from client [%s].", client.getClientId()); + this.event.client(client); + if (clientSession.getUserSession() != null) { this.event.session(clientSession.getUserSession()); } - } else { - validateClientPermissions(clientCode, providerId); } if (isDebugEnabled()) { @@ -509,19 +505,6 @@ public class IdentityBrokerService { throw new IdentityBrokerException("Configuration for identity provider [" + providerId + "] not found."); } - private void validateClientPermissions(ClientSessionCode clientSessionCode, String providerId) { - ClientSessionModel clientSession = clientSessionCode.getClientSession(); - ClientModel clientModel = clientSession.getClient(); - - if (clientModel == null) { - throw new IdentityBrokerException("Invalid client."); - } - - if (!clientModel.hasIdentityProvider(providerId)) { - throw new IdentityBrokerException("Client [" + clientModel.getClientId() + "] not authorized to authenticate with identity provider [" + providerId + "]."); - } - } - private UserModel createUser(FederatedIdentity updatedIdentity) { FederatedIdentityModel federatedIdentityModel = new FederatedIdentityModel(updatedIdentity.getIdentityProviderId(), updatedIdentity.getId(), updatedIdentity.getUsername(), updatedIdentity.getToken()); 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 e8ec27e80f..59a0cf3026 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,7 +1,7 @@ package org.keycloak.services.resources.admin; +import org.jboss.logging.Logger; import org.jboss.resteasy.annotations.cache.NoCache; -import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.broker.provider.IdentityProvider; import org.keycloak.broker.provider.IdentityProviderFactory; import org.keycloak.models.ClientIdentityProviderMappingModel; @@ -22,22 +22,21 @@ import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.PUT; import javax.ws.rs.Path; -import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * @author Pedro Igor */ public class IdentityProviderResource { + private static Logger logger = Logger.getLogger(IdentityProviderResource.class); + private final RealmAuth auth; private final RealmModel realm; private final KeycloakSession session; @@ -62,20 +61,66 @@ public class IdentityProviderResource { public Response delete() { this.auth.requireManage(); removeClientIdentityProviders(this.realm.getApplications(), this.identityProviderModel); - removeClientIdentityProviders(this.realm.getApplications(), this.identityProviderModel); + removeClientIdentityProviders(this.realm.getOAuthClients(), this.identityProviderModel); this.realm.removeIdentityProviderById(this.identityProviderModel.getId()); return Response.noContent().build(); } @PUT @Consumes("application/json") - public Response update(IdentityProviderRepresentation model) { + public Response update(IdentityProviderRepresentation providerRep) { try { this.auth.requireManage(); - this.realm.updateIdentityProvider(RepresentationToModel.toModel(model)); + + String internalId = providerRep.getInternalId(); + String newProviderId = providerRep.getId(); + String oldProviderId = getProviderIdByInternalId(this.realm, internalId); + + this.realm.updateIdentityProvider(RepresentationToModel.toModel(providerRep)); + + if (oldProviderId != null && !oldProviderId.equals(newProviderId)) { + + // User changed the ID (alias) of identity provider. We must update all clients + logger.info("Changing identityProviderMapping in all clients. oldProviderId=" + oldProviderId + ", newProviderId=" + newProviderId); + + updateClientsAfterProviderAliasChange(this.realm.getApplications(), oldProviderId, newProviderId); + updateClientsAfterProviderAliasChange(this.realm.getOAuthClients(), oldProviderId, newProviderId); + } + return Response.noContent().build(); } catch (ModelDuplicateException e) { - return Flows.errors().exists("Identity Provider " + model.getId() + " already exists"); + return Flows.errors().exists("Identity Provider " + providerRep.getId() + " already exists"); + } + } + + // return ID of IdentityProvider from realm based on internalId of this provider + private String getProviderIdByInternalId(RealmModel realm, String providerInternalId) { + List providerModels = realm.getIdentityProviders(); + for (IdentityProviderModel providerModel : providerModels) { + if (providerModel.getInternalId().equals(providerInternalId)) { + return providerModel.getId(); + } + } + + return null; + } + + private void updateClientsAfterProviderAliasChange(List clients, String oldProviderId, String newProviderId) { + for (ClientModel client : clients) { + List clientIdentityProviders = client.getIdentityProviders(); + boolean found = true; + + for (ClientIdentityProviderMappingModel mappingModel : clientIdentityProviders) { + if (mappingModel.getIdentityProvider().equals(oldProviderId)) { + mappingModel.setIdentityProvider(newProviderId); + found = true; + break; + } + } + + if (found) { + client.updateIdentityProviders(clientIdentityProviders); + } } } @@ -113,10 +158,10 @@ public class IdentityProviderResource { for (ClientIdentityProviderMappingModel providerMappingModel : new ArrayList(identityProviders)) { if (providerMappingModel.getIdentityProvider().equals(identityProvider.getId())) { identityProviders.remove(providerMappingModel); + clientModel.updateIdentityProviders(identityProviders); + break; } } - - clientModel.updateAllowedIdentityProviders(identityProviders); } } 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 b59ea609c0..b2d4ebf7b4 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 @@ -94,9 +94,6 @@ 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"); @@ -220,17 +217,4 @@ public class IdentityProvidersResource { return allProviders; } - - private void updateClientIdentityProviders(List clients, IdentityProviderRepresentation identityProvider) { - for (ClientModel clientModel : clients) { - List allowedIdentityProviders = clientModel.getIdentityProviders(); - ClientIdentityProviderMappingModel providerMappingModel = new ClientIdentityProviderMappingModel(); - - providerMappingModel.setIdentityProvider(identityProvider.getId()); - - allowedIdentityProviders.add(providerMappingModel); - - 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 5b66f7f1df..83da549645 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 @@ -62,6 +62,7 @@ import javax.ws.rs.core.UriBuilder; import java.io.IOException; import java.net.URI; +import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -226,40 +227,30 @@ public abstract class AbstractIdentityProviderTest { } @Test - public void testDisabledForApplication() { + public void testProviderOnLoginPage() { IdentityProviderModel identityProviderModel = getIdentityProviderModel(); RealmModel realm = getRealm(); ApplicationModel applicationModel = realm.getApplicationByName("test-app"); - List allowedIdentityProviders = applicationModel.getIdentityProviders(); - ClientIdentityProviderMappingModel mapping = null; - for (ClientIdentityProviderMappingModel model : allowedIdentityProviders) { - if (model.getIdentityProvider().equals(identityProviderModel.getId())) { - mapping = model; - } - } - - assertNotNull(mapping); - - allowedIdentityProviders.remove(mapping); + // This client doesn't have any specific identity providers settings + ClientModel client2 = realm.findClient("test-app"); + assertEquals(0, client2.getIdentityProviders().size()); + // Provider button is available on login page 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/auth")); + this.driver.findElement(By.className(getProviderId())); - try { - this.driver.findElement(By.className(getProviderId())); - fail("Provider [" + getProviderId() + "] not disabled."); - } catch (NoSuchElementException nsee) { - - } - - allowedIdentityProviders.add(mapping); - - applicationModel.updateAllowedIdentityProviders(allowedIdentityProviders); + // Add identityProvider to client model + List appIdentityProviders = new ArrayList(); + ClientIdentityProviderMappingModel mapping = new ClientIdentityProviderMappingModel(); + mapping.setIdentityProvider(getProviderId()); + mapping.setRetrieveToken(true); + appIdentityProviders.add(mapping); + applicationModel.updateIdentityProviders(appIdentityProviders); + // Provider button still available on login page this.driver.navigate().to("http://localhost:8081/test-app/"); - this.driver.findElement(By.className(getProviderId())); } @@ -477,16 +468,16 @@ public abstract class AbstractIdentityProviderTest { assertTrue(oauth.getCurrentQuery().containsKey(OAuth2Constants.CODE)); ClientModel clientModel = getRealm().findClient("third-party"); - ClientIdentityProviderMappingModel providerMappingModel = null; - - for (ClientIdentityProviderMappingModel identityProviderMappingModel : clientModel.getIdentityProviders()) { - if (identityProviderMappingModel.getIdentityProvider().equals(getProviderId())) { - providerMappingModel = identityProviderMappingModel; - break; - } - } + assertEquals(0, clientModel.getIdentityProviders().size()); + ClientIdentityProviderMappingModel providerMappingModel = new ClientIdentityProviderMappingModel(); + providerMappingModel.setIdentityProvider(getProviderId()); providerMappingModel.setRetrieveToken(true); + List providerMappingModels = new ArrayList(); + providerMappingModels.add(providerMappingModel); + clientModel.updateIdentityProviders(providerMappingModels); + brokerServerRule.stopSession(session, true); + session = brokerServerRule.startSession(); AccessTokenResponse accessToken = oauth.doAccessTokenRequest(oauth.getCurrentQuery().get(OAuth2Constants.CODE), "password"); URI tokenEndpointUrl = Urls.identityProviderRetrieveToken(BASE_URI, getProviderId(), getRealm().getName()); 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 fe4febd40c..3906666673 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/ImportIdentityProviderTest.java @@ -133,7 +133,7 @@ public class ImportIdentityProviderTest extends AbstractIdentityProviderModelTes identityProviders.remove(identityProviderMappingModel); - client.updateAllowedIdentityProviders(identityProviders); + client.updateIdentityProviders(identityProviders); client = realm.findClientById(client.getId()); identityProviders = client.getIdentityProviders();