From b1d341155d47f395249bf0c79d8dabe84fe616be Mon Sep 17 00:00:00 2001 From: Stan Silvert Date: Thu, 5 Mar 2015 15:50:16 -0500 Subject: [PATCH] Fix AdminApiTest. Fix distribution. Try to prevent InMemoryModel leaks. --- distribution/modules/build.xml | 5 ++++ .../keycloak-model-file/main/module.xml | 20 +++++++++++++ .../keycloak-services/main/module.xml | 1 + .../WEB-INF/jboss-deployment-structure.xml | 1 + .../models/file/FileRealmProvider.java | 4 ++- .../models/file/FileUserProvider.java | 28 ++++++++++++++++--- .../keycloak/models/file/InMemoryModel.java | 25 +++++++++++++---- .../file/adapter/ApplicationAdapter.java | 1 + .../models/file/adapter/RealmAdapter.java | 2 +- .../models/file/adapter/RoleAdapter.java | 1 + testsuite/integration/pom.xml | 1 - .../resources/META-INF/keycloak-server.json | 4 +-- .../testsuite/broker/BrokerKeyCloakRule.java | 6 +++- .../broker/IdentityProviderHintTest.java | 5 +++- .../OIDCKeyCloakServerBrokerBasicTest.java | 5 +++- .../SAMLKeyCloakServerBrokerBasicTest.java | 5 +++- ...KeyCloakServerBrokerWithSignatureTest.java | 5 +++- 17 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 distribution/modules/src/main/resources/modules/org/keycloak/keycloak-model-file/main/module.xml diff --git a/distribution/modules/build.xml b/distribution/modules/build.xml index efb3ba4be3..43f9337ebb 100755 --- a/distribution/modules/build.xml +++ b/distribution/modules/build.xml @@ -254,6 +254,11 @@ + + + + + diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-model-file/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-model-file/main/module.xml new file mode 100644 index 0000000000..7652c83689 --- /dev/null +++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-model-file/main/module.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml index 74e67a7320..8181ba8b87 100755 --- a/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml +++ b/distribution/modules/src/main/resources/modules/org/keycloak/keycloak-services/main/module.xml @@ -42,6 +42,7 @@ + diff --git a/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml b/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml index 9818d93c2b..1051a617fc 100755 --- a/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml +++ b/distribution/subsystem-war/src/main/webapp/WEB-INF/jboss-deployment-structure.xml @@ -33,6 +33,7 @@ + diff --git a/model/file/src/main/java/org/keycloak/models/file/FileRealmProvider.java b/model/file/src/main/java/org/keycloak/models/file/FileRealmProvider.java index a70dd39b03..4ef446ebca 100644 --- a/model/file/src/main/java/org/keycloak/models/file/FileRealmProvider.java +++ b/model/file/src/main/java/org/keycloak/models/file/FileRealmProvider.java @@ -32,7 +32,7 @@ import org.keycloak.models.entities.RealmEntity; /** * Realm Provider for JSON persistence. - * + * * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc. */ public class FileRealmProvider implements RealmProvider { @@ -42,6 +42,7 @@ public class FileRealmProvider implements RealmProvider { public FileRealmProvider(KeycloakSession session, InMemoryModel inMemoryModel) { this.session = session; + session.enlistForClose(this); this.inMemoryModel = inMemoryModel; } @@ -86,6 +87,7 @@ public class FileRealmProvider implements RealmProvider { @Override public void close() { + inMemoryModel.sessionClosed(session); } @Override diff --git a/model/file/src/main/java/org/keycloak/models/file/FileUserProvider.java b/model/file/src/main/java/org/keycloak/models/file/FileUserProvider.java index 099167d2ce..1a16f42112 100644 --- a/model/file/src/main/java/org/keycloak/models/file/FileUserProvider.java +++ b/model/file/src/main/java/org/keycloak/models/file/FileUserProvider.java @@ -53,11 +53,13 @@ public class FileUserProvider implements UserProvider { public FileUserProvider(KeycloakSession session, InMemoryModel inMemoryModel) { this.session = session; + session.enlistForClose(this); this.inMemoryModel = inMemoryModel; } @Override public void close() { + inMemoryModel.sessionClosed(session); } @Override @@ -219,8 +221,7 @@ public class FileUserProvider implements UserProvider { @Override public Set getFederatedIdentities(UserModel userModel, RealmModel realm) { - UserModel user = getUserById(userModel.getId(), realm); - UserEntity userEntity = ((UserAdapter) user).getUserEntity(); + UserEntity userEntity = ((UserAdapter) userModel).getUserEntity(); List linkEntities = userEntity.getFederatedIdentities(); if (linkEntities == null) { @@ -391,12 +392,31 @@ public class FileUserProvider implements UserProvider { @Override public void updateFederatedIdentity(RealmModel realm, UserModel federatedUser, FederatedIdentityModel federatedIdentityModel) { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + federatedUser = getUserById(federatedUser.getId(), realm); + UserEntity userEntity = ((UserAdapter) federatedUser).getUserEntity(); + FederatedIdentityEntity federatedIdentityEntity = findFederatedIdentityLink(userEntity, federatedIdentityModel.getIdentityProvider()); + + federatedIdentityEntity.setToken(federatedIdentityModel.getToken()); + } + + private FederatedIdentityEntity findFederatedIdentityLink(UserEntity userEntity, String identityProvider) { + List linkEntities = userEntity.getFederatedIdentities(); + if (linkEntities == null) { + return null; + } + + for (FederatedIdentityEntity federatedIdentityEntity : linkEntities) { + if (federatedIdentityEntity.getIdentityProvider().equals(identityProvider)) { + return federatedIdentityEntity; + } + } + return null; } @Override public CredentialValidationOutput validCredentials(RealmModel realm, UserCredentialModel... input) { - throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + //throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + return null; // not supported yet } } diff --git a/model/file/src/main/java/org/keycloak/models/file/InMemoryModel.java b/model/file/src/main/java/org/keycloak/models/file/InMemoryModel.java index 35205ce5a3..db331888ca 100644 --- a/model/file/src/main/java/org/keycloak/models/file/InMemoryModel.java +++ b/model/file/src/main/java/org/keycloak/models/file/InMemoryModel.java @@ -41,7 +41,8 @@ import org.keycloak.util.JsonSerialization; /** * This class provides an in-memory copy of the entire model for each * Keycloak session. At the start of the session, the model is read - * from JSON. When the session's transaction ends, the model is written. + * from JSON. When the session's transaction ends, the model is written back + * out. * * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc. */ @@ -83,6 +84,7 @@ public class InMemoryModel implements KeycloakTransaction { allModels.put(session, model); session.getTransaction().enlist(model); model.readModelFile(); + logger.info("Added session " + session.hashCode() + " total sessions=" + allModels.size()); } return model; @@ -104,6 +106,7 @@ public class InMemoryModel implements KeycloakTransaction { } catch (IOException ioe) { logger.error("Unable to read model file " + kcdata.getAbsolutePath(), ioe); } finally { + logger.info("Read model file for session=" + session.hashCode()); try { if (fis != null) fis.close(); } catch (IOException e) { @@ -121,6 +124,7 @@ public class InMemoryModel implements KeycloakTransaction { } catch (IOException e) { logger.error("Unable to write model file " + keycloakModelFile.getAbsolutePath(), e); } finally { + logger.info("Wrote model file for session=" + session.hashCode()); try { if (outStream != null) outStream.close(); } catch (IOException e) { @@ -195,17 +199,25 @@ public class InMemoryModel implements KeycloakTransaction { return (realmUsers(realmId).remove(userId) != null); } + void sessionClosed(KeycloakSession session) { + synchronized (allModels) { + allModels.remove(session); + logger.info("Removed session " + session.hashCode()); + logger.info("sessionClosed: Session count=" + allModels.size()); + } + } + @Override public void begin() { } // commitCount is used for debugging. This allows you to easily run a test // to a particular point and then examine the JSON file. - // private static int commitCount = 0; + private static int commitCount = 0; @Override public void commit() { -// commitCount++; + commitCount++; synchronized (allModels) { // in case commit was somehow called twice on the same session if (!allModels.containsKey(session)) return; @@ -214,10 +226,12 @@ public class InMemoryModel implements KeycloakTransaction { writeModelFile(); } finally { allModels.remove(session); - // System.out.println("*** commitCount=" + commitCount); + logger.info("Removed session " + session.hashCode()); + logger.info("*** commitCount=" + commitCount); + logger.info("commit(): Session count=" + allModels.size()); } - // if (commitCount == 61) System.exit(0); + // if (commitCount == 16) {Thread.dumpStack();System.exit(0);} } } @@ -225,6 +239,7 @@ public class InMemoryModel implements KeycloakTransaction { public void rollback() { synchronized (allModels) { allModels.remove(session); + System.out.println("rollback(): Session count=" + allModels.size()); } } diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/ApplicationAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/ApplicationAdapter.java index f2a83f2e43..813d11fe9e 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/ApplicationAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/ApplicationAdapter.java @@ -77,6 +77,7 @@ public class ApplicationAdapter extends ClientAdapter implements ApplicationMode private boolean appNameExists(String name) { for (ApplicationModel app : realm.getApplications()) { + if (app == this) continue; if (app.getName().equals(name)) return true; } diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/RealmAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/RealmAdapter.java index b26da21f8f..2e4b07a07f 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/RealmAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/RealmAdapter.java @@ -1033,7 +1033,7 @@ public class RealmAdapter implements RealmModel { @Override public boolean isIdentityFederationEnabled() { //TODO: not sure if we will support identity federation storage for file - return true; + return getIdentityProviders() != null && !getIdentityProviders().isEmpty(); } @Override diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/RoleAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/RoleAdapter.java index d5f23bfcd6..28a2f9001c 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/RoleAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/RoleAdapter.java @@ -73,6 +73,7 @@ public class RoleAdapter implements RoleModel { @Override public void setName(String name) { RealmAdapter realmAdapter = (RealmAdapter)realm; + if (role.getName().equals(name)) return; if (realmAdapter.hasRoleWithName(name)) throw new ModelDuplicateException("Role name " + name + " already exists."); role.setName(name); } diff --git a/testsuite/integration/pom.xml b/testsuite/integration/pom.xml index 83677b8cf6..cc9b02a3a2 100755 --- a/testsuite/integration/pom.xml +++ b/testsuite/integration/pom.xml @@ -406,7 +406,6 @@ maven-surefire-plugin - **/AdminAPITest.java **/ExportImportTest.java **/SyncProvidersTest.java diff --git a/testsuite/integration/src/main/resources/META-INF/keycloak-server.json b/testsuite/integration/src/main/resources/META-INF/keycloak-server.json index 2eaff74307..5b1c73540b 100755 --- a/testsuite/integration/src/main/resources/META-INF/keycloak-server.json +++ b/testsuite/integration/src/main/resources/META-INF/keycloak-server.json @@ -8,7 +8,7 @@ }, "realm": { - "provider": "${keycloak.realm.provider:file}", + "provider": "${keycloak.realm.provider:jpa}", "file" : { "directory" : ".", "fileName" : "kcdata.json" @@ -16,7 +16,7 @@ }, "user": { - "provider": "${keycloak.user.provider:file}" + "provider": "${keycloak.user.provider:jpa}" }, "userSessions": { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/BrokerKeyCloakRule.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/BrokerKeyCloakRule.java index deb261143f..32d1f84b92 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/BrokerKeyCloakRule.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/BrokerKeyCloakRule.java @@ -24,6 +24,8 @@ import org.keycloak.testsuite.broker.util.UserSessionStatusServlet; import org.keycloak.testsuite.rule.AbstractKeycloakRule; import java.net.URL; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.testutils.KeycloakServer; /** * @author pedroigor @@ -32,7 +34,9 @@ public class BrokerKeyCloakRule extends AbstractKeycloakRule { @Override protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { - server.importRealm(getClass().getResourceAsStream("/broker-test/test-realm-with-broker.json")); + //server.importRealm(getClass().getResourceAsStream("/broker-test/test-realm-with-broker.json")); + RealmRepresentation realmWithBroker = KeycloakServer.loadJson(getClass().getResourceAsStream("/broker-test/test-realm-with-broker.json"), RealmRepresentation.class); + manager.importRealm(realmWithBroker); URL url = getClass().getResource("/broker-test/test-app-keycloak.json"); deployApplication("test-app", "/test-app", UserSessionStatusServlet.class, url.getPath(), "manager"); deployApplication("test-app-allowed-providers", "/test-app-allowed-providers", UserSessionStatusServlet.class, url.getPath(), "manager"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/IdentityProviderHintTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/IdentityProviderHintTest.java index b46617eac4..d9c67307ce 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/IdentityProviderHintTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/IdentityProviderHintTest.java @@ -17,6 +17,7 @@ import org.openqa.selenium.WebDriver; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import org.keycloak.representations.idm.RealmRepresentation; /** * @author pedroigor @@ -36,7 +37,9 @@ public class IdentityProviderHintTest { @Override protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { - server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json")); + //server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json")); + RealmRepresentation realmWithOIDC = KeycloakServer.loadJson(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json"), RealmRepresentation.class); + manager.importRealm(realmWithOIDC); } }; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index 66e4b7183d..9b84345e25 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -15,6 +15,7 @@ import java.io.IOException; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import org.keycloak.representations.idm.RealmRepresentation; /** * @author pedroigor @@ -31,7 +32,9 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractIdentityProviderT @Override protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { - server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json")); + //server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json")); + RealmRepresentation realmWithOIDC = KeycloakServer.loadJson(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json"), RealmRepresentation.class); + manager.importRealm(realmWithOIDC); } }; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java index 77836655bd..50f7b9885e 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; +import org.keycloak.representations.idm.RealmRepresentation; /** * @author pedroigor @@ -35,7 +36,9 @@ public class SAMLKeyCloakServerBrokerBasicTest extends AbstractIdentityProviderT @Override protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { - server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml.json")); + //server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml.json")); + RealmRepresentation realmWithSAML = KeycloakServer.loadJson(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml.json"), RealmRepresentation.class); + manager.importRealm(realmWithSAML); } }; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java index e387097316..d1886f9e9c 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; +import org.keycloak.representations.idm.RealmRepresentation; /** * @author pedroigor @@ -35,7 +36,9 @@ public class SAMLKeyCloakServerBrokerWithSignatureTest extends AbstractIdentityP @Override protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) { - server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml-with-signature.json")); + //server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml-with-signature.json")); + RealmRepresentation realmWithOIDC = KeycloakServer.loadJson(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-saml-with-signature.json"), RealmRepresentation.class); + manager.importRealm(realmWithOIDC); } };