From 6587cd2478697e9afdb51c5c4deb65ab0cac9cc9 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Mon, 5 Dec 2016 17:51:06 -0500 Subject: [PATCH] KEYCLOAK-3620 --- .../org/keycloak/models/jpa/RealmAdapter.java | 22 ++- .../mongo/keycloak/adapters/RealmAdapter.java | 20 ++- .../models/utils/ModelToRepresentation.java | 11 +- .../resources/admin/ComponentResource.java | 8 +- .../keycloak/storage/UserStorageManager.java | 15 +- .../storage/BrokenUserStorageTest.java | 138 ++++++++++++++++++ 6 files changed, 194 insertions(+), 20 deletions(-) create mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/BrokenUserStorageTest.java diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 417a38bca2..f11ba9090d 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -1749,14 +1749,28 @@ public class RealmAdapter implements RealmModel, JpaModel { return model; } + /** + * This just exists for testing purposes + * + */ + public static final String COMPONENT_PROVIDER_EXISTS_DISABLED = "component.provider.exists.disabled"; + @Override public ComponentModel importComponentModel(ComponentModel model) { - ComponentFactory componentFactory = ComponentUtil.getComponentFactory(session, model); - if (componentFactory == null) { - throw new IllegalArgumentException("Invalid component type"); + ComponentFactory componentFactory = null; + try { + componentFactory = ComponentUtil.getComponentFactory(session, model); + if (componentFactory == null && System.getProperty(COMPONENT_PROVIDER_EXISTS_DISABLED) == null) { + throw new IllegalArgumentException("Invalid component type"); + } + componentFactory.validateConfiguration(session, this, model); + } catch (Exception e) { + if (System.getProperty(COMPONENT_PROVIDER_EXISTS_DISABLED) == null) { + throw e; + } + } - componentFactory.validateConfiguration(session, this, model); ComponentEntity c = new ComponentEntity(); if (model.getId() == null) { diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index f79bbdacd5..cf8fd56d26 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -21,6 +21,7 @@ import com.mongodb.DBObject; import com.mongodb.QueryBuilder; import org.keycloak.common.enums.SslRequired; import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.component.ComponentFactory; import org.keycloak.component.ComponentModel; import org.keycloak.connections.mongo.api.context.MongoStoreInvocationContext; import org.keycloak.models.AuthenticationExecutionModel; @@ -1675,10 +1676,27 @@ public class RealmAdapter extends AbstractMongoAdapter impleme return model; } + /** + * This just exists for testing purposes + * + */ + public static final String COMPONENT_PROVIDER_EXISTS_DISABLED = "component.provider.exists.disabled"; + @Override public ComponentModel importComponentModel(ComponentModel model) { - ComponentUtil.getComponentFactory(session, model).validateConfiguration(session, this, model); + ComponentFactory componentFactory = null; + try { + componentFactory = ComponentUtil.getComponentFactory(session, model); + if (componentFactory == null && System.getProperty(COMPONENT_PROVIDER_EXISTS_DISABLED) == null) { + throw new IllegalArgumentException("Invalid component type"); + } + componentFactory.validateConfiguration(session, this, model); + } catch (Exception e) { + if (System.getProperty(COMPONENT_PROVIDER_EXISTS_DISABLED) == null) { + throw e; + } + } ComponentEntity entity = new ComponentEntity(); if (model.getId() == null) { entity.setId(KeycloakModelUtils.generateId()); diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 26a30b487a..27dd6dc64f 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -750,6 +750,14 @@ public class ModelToRepresentation { } public static ComponentRepresentation toRepresentation(KeycloakSession session, ComponentModel component, boolean internal) { + ComponentRepresentation rep = toRepresentationWithoutConfig(component); + if (!internal) { + rep = StripSecretsUtils.strip(session, rep); + } + return rep; + } + + public static ComponentRepresentation toRepresentationWithoutConfig(ComponentModel component) { ComponentRepresentation rep = new ComponentRepresentation(); rep.setId(component.getId()); rep.setName(component.getName()); @@ -758,9 +766,6 @@ public class ModelToRepresentation { rep.setSubType(component.getSubType()); rep.setParentId(component.getParentId()); rep.setConfig(new MultivaluedHashMap<>(component.getConfig())); - if (!internal) { - rep = StripSecretsUtils.strip(session, rep); - } return rep; } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java index 1b505239ae..9bb56546da 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java @@ -118,7 +118,13 @@ public class ComponentResource { List reps = new LinkedList<>(); for (ComponentModel component : components) { if (name != null && !name.equals(component.getName())) continue; - ComponentRepresentation rep = ModelToRepresentation.toRepresentation(session, component, false); + ComponentRepresentation rep = null; + try { + rep = ModelToRepresentation.toRepresentation(session, component, false); + } catch (Exception e) { + logger.error("Failed to get component list for component model" + component.getName() + "of realm " + realm.getName()); + rep = ModelToRepresentation.toRepresentationWithoutConfig(component); + } reps.add(rep); } return reps; diff --git a/services/src/main/java/org/keycloak/storage/UserStorageManager.java b/services/src/main/java/org/keycloak/storage/UserStorageManager.java index 4924f19fee..e1a269c65d 100755 --- a/services/src/main/java/org/keycloak/storage/UserStorageManager.java +++ b/services/src/main/java/org/keycloak/storage/UserStorageManager.java @@ -74,17 +74,6 @@ public class UserStorageManager implements UserProvider, OnUserCache { return realm.getUserStorageProviders(); } - public static T getFirstStorageProvider(KeycloakSession session, RealmModel realm, Class type) { - for (UserStorageProviderModel model : getStorageProviders(realm)) { - UserStorageProviderFactory factory = (UserStorageProviderFactory)session.getKeycloakSessionFactory().getProviderFactory(UserStorageProvider.class, model.getProviderId()); - - if (Types.supports(type, factory, UserStorageProviderFactory.class)) { - return type.cast(getStorageProviderInstance(session, model, factory)); - } - } - return null; - } - public static UserStorageProvider getStorageProviderInstance(KeycloakSession session, UserStorageProviderModel model, UserStorageProviderFactory factory) { UserStorageProvider instance = (UserStorageProvider)session.getAttribute(model.getId()); if (instance != null) return instance; @@ -99,6 +88,10 @@ public class UserStorageManager implements UserProvider, OnUserCache { List list = new LinkedList<>(); for (UserStorageProviderModel model : getStorageProviders(realm)) { UserStorageProviderFactory factory = (UserStorageProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(UserStorageProvider.class, model.getProviderId()); + if (factory == null) { + logger.warnv("Configured UserStorageProvider {0} of provider id {1} does not exist in realm {2}", model.getName(), model.getProviderId(), realm.getName()); + continue; + } if (Types.supports(type, factory, UserStorageProviderFactory.class)) { list.add(type.cast(getStorageProviderInstance(session, model, factory))); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/BrokenUserStorageTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/BrokenUserStorageTest.java new file mode 100644 index 0000000000..02c12f677d --- /dev/null +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/BrokenUserStorageTest.java @@ -0,0 +1,138 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.testsuite.federation.storage; + +import org.junit.After; +import org.junit.Assert; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.common.util.Time; +import org.keycloak.component.ComponentModel; +import org.keycloak.models.Constants; +import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; +import org.keycloak.models.UserCredentialModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.cache.CachedUserModel; +import org.keycloak.models.cache.infinispan.UserAdapter; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.services.managers.RealmManager; +import org.keycloak.storage.StorageId; +import org.keycloak.storage.UserStorageProvider; +import org.keycloak.storage.UserStorageProviderModel; +import org.keycloak.testsuite.ApplicationServlet; +import org.keycloak.testsuite.OAuthClient; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.rule.KeycloakRule; +import org.keycloak.testsuite.rule.WebResource; +import org.keycloak.testsuite.rule.WebRule; +import org.openqa.selenium.WebDriver; + +import java.util.Calendar; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * KEYCLOAK-3903 and KEYCLOAK-3620 + * + * @author Bill Burke + * @version $Revision: 1 $ + */ +public class BrokenUserStorageTest { + @ClassRule + public static KeycloakRule keycloakRule = new KeycloakRule(); + + @Rule + public WebRule webRule = new WebRule(this); + + @WebResource + protected OAuthClient oauth; + + @WebResource + protected WebDriver driver; + + @WebResource + protected AppPage appPage; + + @WebResource + protected LoginPage loginPage; + + private void loginSuccessAndLogout(String username, String password) { + loginPage.open(); + loginPage.login(username, password); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + oauth.openLogout(); + } + protected String AUTH_SERVER_URL = "http://localhost:8081/auth"; + + @Test + public void testBootWithBadProviderId() throws Exception { + KeycloakSession session = keycloakRule.startSession(); + // set this system property + System.setProperty("component.provider.exists.disabled", "true"); + RealmModel realm = session.realms().getRealmByName("master"); + String masterId = realm.getId(); + UserStorageProviderModel model; + model = new UserStorageProviderModel(); + model.setName("bad-provider-id"); + model.setPriority(2); + model.setParentId(realm.getId()); + model.setProviderId("error"); + ComponentModel component = realm.importComponentModel(model); + + keycloakRule.stopSession(session, true); + + keycloakRule.restartServer(); + keycloakRule.deployServlet("app", "/app", ApplicationServlet.class); + + loginSuccessAndLogout("test-user@localhost", "password"); + + // make sure we can list components and delete provider as this is an admin console operation + + Keycloak keycloakAdmin = Keycloak.getInstance(AUTH_SERVER_URL, "master", "admin", "admin", Constants.ADMIN_CLI_CLIENT_ID); + RealmResource master = keycloakAdmin.realms().realm("master"); + List components = master.components().query(masterId, UserStorageProvider.class.getName()); + boolean found = false; + for (ComponentRepresentation rep : components) { + if (rep.getName().equals("bad-provider-id")) { + found = true; + } + } + Assert.assertTrue(found); + + master.components().component(component.getId()).remove(); + + List components2 = master.components().query(masterId, UserStorageProvider.class.getName()); + Assert.assertEquals(components.size() - 1, components2.size()); + } + + @After + public void resetTimeoffset() { + Time.setOffset(0); + + } + + }