From c9cc1896027c3e6a2d4e7855871c9a4d9914cbdc Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 12 Feb 2016 16:28:07 -0500 Subject: [PATCH] make RealmModel unmodifiable collections --- .../models/cache/infinispan/RealmAdapter.java | 72 ++++------ .../org/keycloak/models/jpa/RealmAdapter.java | 136 +++++++++++------- .../mongo/keycloak/adapters/RealmAdapter.java | 115 +++++++++------ .../models/cache/entities/CachedRealm.java | 68 ++++++--- .../models/utils/ModelToRepresentation.java | 3 + .../testsuite/forms/ResetPasswordTest.java | 14 +- 6 files changed, 250 insertions(+), 158 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index 14e2850752..6a61851683 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -502,11 +502,8 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredCredentials() { - - List copy = new LinkedList(); - if (updated != null) copy.addAll(updated.getRequiredCredentials()); - else copy.addAll(cached.getRequiredCredentials()); - return copy; + if (updated != null) return updated.getRequiredCredentials(); + return cached.getRequiredCredentials(); } @Override @@ -548,11 +545,13 @@ public class RealmAdapter implements RealmModel { @Override public List getDefaultGroups() { + if (updated != null) return updated.getDefaultGroups(); + List defaultGroups = new LinkedList<>(); for (String id : cached.getDefaultGroups()) { defaultGroups.add(cacheSession.getGroupById(id, this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @@ -599,13 +598,13 @@ public class RealmAdapter implements RealmModel { } map.put(model.getClientId(), model); } - return map; + return Collections.unmodifiableMap(map); } @Override public List getClients() { if (updated != null) return updated.getClients(); - List apps = new LinkedList(); + List apps = new LinkedList<>(); for (String id : cached.getClients().values()) { ClientModel model = cacheSession.getClientById(id, this); if (model == null) { @@ -613,7 +612,7 @@ public class RealmAdapter implements RealmModel { } apps.add(model); } - return apps; + return Collections.unmodifiableList(apps); } @@ -691,6 +690,7 @@ public class RealmAdapter implements RealmModel { @Override public IdentityProviderModel getIdentityProviderByAlias(String alias) { + if (updated != null) return updated.getIdentityProviderByAlias(alias); for (IdentityProviderModel identityProviderModel : getIdentityProviders()) { if (identityProviderModel.getAlias().equals(alias)) { return identityProviderModel; @@ -941,7 +941,7 @@ public class RealmAdapter implements RealmModel { if (roleById == null) continue; roles.add(roleById); } - return roles; + return Collections.unmodifiableSet(roles); } @Override @@ -1003,13 +1003,7 @@ public class RealmAdapter implements RealmModel { @Override public Set getIdentityProviderMappers() { if (updated != null) return updated.getIdentityProviderMappers(); - Set mappings = new HashSet<>(); - for (List models : cached.getIdentityProviderMappers().values()) { - for (IdentityProviderMapperModel model : models) { - mappings.add(model); - } - } - return mappings; + return cached.getIdentityProviderMapperSet(); } @Override @@ -1020,7 +1014,7 @@ public class RealmAdapter implements RealmModel { for (IdentityProviderMapperModel entity : list) { mappings.add(entity); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1066,13 +1060,7 @@ public class RealmAdapter implements RealmModel { @Override public Set getUserFederationMappers() { if (updated != null) return updated.getUserFederationMappers(); - Set mappers = new HashSet(); - for (List models : cached.getUserFederationMappers().values()) { - for (UserFederationMapperModel model : models) { - mappers.add(model); - } - } - return mappers; + return cached.getUserFederationMapperSet(); } @Override @@ -1083,7 +1071,7 @@ public class RealmAdapter implements RealmModel { for (UserFederationMapperModel entity : list) { mappers.add(entity); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -1192,9 +1180,7 @@ public class RealmAdapter implements RealmModel { @Override public List getAuthenticationFlows() { if (updated != null) return updated.getAuthenticationFlows(); - List models = new ArrayList<>(); - models.addAll(cached.getAuthenticationFlows().values()); - return models; + return cached.getAuthenticationFlowList(); } @Override @@ -1281,7 +1267,7 @@ public class RealmAdapter implements RealmModel { if (updated != null) return updated.getAuthenticatorConfigs(); List models = new ArrayList<>(); models.addAll(cached.getAuthenticatorConfigs().values()); - return models; + return Collections.unmodifiableList(models); } @Override @@ -1313,9 +1299,7 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredActionProviders() { if (updated != null) return updated.getRequiredActionProviders(); - List models = new ArrayList<>(); - models.addAll(cached.getRequiredActionProviders().values()); - return models; + return cached.getRequiredActionProviderList(); } @Override @@ -1366,20 +1350,20 @@ public class RealmAdapter implements RealmModel { if (group == null) continue; list.add(group); } - return list; + return Collections.unmodifiableList(list); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParent() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -1416,15 +1400,17 @@ public class RealmAdapter implements RealmModel { @Override public List getClientTemplates() { if (updated != null) return updated.getClientTemplates(); + List clientTemplates = cached.getClientTemplates(); + if (clientTemplates.isEmpty()) return Collections.EMPTY_LIST; List apps = new LinkedList(); - for (String id : cached.getClientTemplates()) { + for (String id : clientTemplates) { ClientTemplateModel model = cacheSession.getClientTemplateById(id, this); if (model == null) { throw new IllegalStateException("Cached clientemplate not found: " + id); } apps.add(model); } - return apps; + return Collections.unmodifiableList(apps); } 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 da66a3ba38..d99dedc28f 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 @@ -608,9 +608,9 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredCredentials() { - List requiredCredentialModels = new ArrayList(); Collection entities = realm.getRequiredCredentials(); - if (entities == null) return requiredCredentialModels; + if (entities == null) return Collections.EMPTY_LIST; + List requiredCredentialModels = new LinkedList<>(); for (RequiredCredentialEntity entity : entities) { RequiredCredentialModel model = new RequiredCredentialModel(); model.setFormLabel(entity.getFormLabel()); @@ -619,19 +619,19 @@ public class RealmAdapter implements RealmModel { model.setInput(entity.isInput()); requiredCredentialModels.add(model); } - return requiredCredentialModels; //To change body of implemented methods use File | Settings | File Templates. + return Collections.unmodifiableList(requiredCredentialModels); } @Override public List getDefaultRoles() { Collection entities = realm.getDefaultRoles(); - List roles = new ArrayList(); - if (entities == null) return roles; + if (entities == null || entities.isEmpty()) return Collections.emptyList(); + List roles = new LinkedList<>(); for (RoleEntity entity : entities) { roles.add(entity.getName()); } - return roles; + return Collections.unmodifiableList(roles); } @Override @@ -685,11 +685,12 @@ public class RealmAdapter implements RealmModel { @Override public List getDefaultGroups() { Collection entities = realm.getDefaultGroups(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List defaultGroups = new LinkedList<>(); for (GroupEntity entity : entities) { defaultGroups.add(session.realms().getGroupById(entity.getId(), this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @Override @@ -722,23 +723,26 @@ public class RealmAdapter implements RealmModel { @Override public Map getClientNameMap() { + List clients = getClients(); + if (clients.isEmpty()) return Collections.EMPTY_MAP; Map map = new HashMap(); - for (ClientModel app : getClients()) { + for (ClientModel app : clients) { map.put(app.getClientId(), app); } - return map; //To change body of implemented methods use File | Settings | File Templates. + return Collections.unmodifiableMap(map); } @Override public List getClients() { - List list = new LinkedList<>(); TypedQuery query = em.createNamedQuery("getClientsByRealm", ClientEntity.class); query.setParameter("realm", realm); List clients = query.getResultList(); + if (clients.isEmpty()) return Collections.EMPTY_LIST; + List list = new LinkedList<>(); for (ClientEntity entity : clients) { list.add(new ClientAdapter(this, em, session, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override @@ -794,13 +798,14 @@ public class RealmAdapter implements RealmModel { @Override public Map getBrowserSecurityHeaders() { Map attributes = getAttributes(); + if (attributes.isEmpty()) return Collections.EMPTY_MAP; Map headers = new HashMap(); for (Map.Entry entry : attributes.entrySet()) { if (entry.getKey().startsWith(BROWSER_HEADER_PREFIX)) { headers.put(entry.getKey().substring(BROWSER_HEADER_PREFIX.length()), entry.getValue()); } } - return headers; + return Collections.unmodifiableMap(headers); } @Override @@ -812,7 +817,9 @@ public class RealmAdapter implements RealmModel { @Override public Map getSmtpConfig() { - return realm.getSmtpConfig(); + Map config = new HashMap(); + config.putAll(realm.getSmtpConfig()); + return Collections.unmodifiableMap(config); } @Override @@ -824,6 +831,7 @@ public class RealmAdapter implements RealmModel { @Override public List getUserFederationProviders() { List entities = realm.getUserFederationProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List copy = new ArrayList(); for (UserFederationProviderEntity entity : entities) { copy.add(entity); @@ -843,7 +851,7 @@ public class RealmAdapter implements RealmModel { entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -1062,13 +1070,13 @@ public class RealmAdapter implements RealmModel { @Override public Set getRoles() { - Set list = new HashSet(); Collection roles = realm.getRoles(); - if (roles == null) return list; + if (roles == null) return Collections.EMPTY_SET; + Set list = new HashSet(); for (RoleEntity entity : roles) { list.add(new RoleAdapter(this, em, entity)); } - return list; + return Collections.unmodifiableSet(list); } @Override @@ -1206,7 +1214,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getEventsListeners() { - return realm.getEventsListeners(); + Set eventsListeners = realm.getEventsListeners(); + if (eventsListeners.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(eventsListeners); + return Collections.unmodifiableSet(copy); } @Override @@ -1217,7 +1229,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getEnabledEventTypes() { - return realm.getEnabledEventTypes(); + Set enabledEventTypes = realm.getEnabledEventTypes(); + if (enabledEventTypes.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(enabledEventTypes); + return Collections.unmodifiableSet(copy); } @Override @@ -1268,15 +1284,20 @@ public class RealmAdapter implements RealmModel { @Override public List getIdentityProviders() { + List entities = realm.getIdentityProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List identityProviders = new ArrayList(); - for (IdentityProviderEntity entity: realm.getIdentityProviders()) { + for (IdentityProviderEntity entity: entities) { IdentityProviderModel identityProviderModel = new IdentityProviderModel(); identityProviderModel.setProviderId(entity.getProviderId()); identityProviderModel.setAlias(entity.getAlias()); identityProviderModel.setInternalId(entity.getInternalId()); - identityProviderModel.setConfig(entity.getConfig()); + Map config = entity.getConfig(); + Map copy = new HashMap<>(); + copy.putAll(config); + identityProviderModel.setConfig(copy); identityProviderModel.setEnabled(entity.isEnabled()); identityProviderModel.setTrustEmail(entity.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(entity.isAuthenticateByDefault()); @@ -1288,7 +1309,7 @@ public class RealmAdapter implements RealmModel { identityProviders.add(identityProviderModel); } - return identityProviders; + return Collections.unmodifiableList(identityProviders); } @Override @@ -1373,7 +1394,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getSupportedLocales() { - return realm.getSupportedLocales(); + Set supportedLocales = realm.getSupportedLocales(); + if (supportedLocales == null || supportedLocales.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(supportedLocales); + return Collections.unmodifiableSet(copy); } @Override @@ -1395,12 +1420,14 @@ public class RealmAdapter implements RealmModel { @Override public Set getIdentityProviderMappers() { + Collection entities = this.realm.getIdentityProviderMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappings = new HashSet(); - for (IdentityProviderMapperEntity entity : this.realm.getIdentityProviderMappers()) { + for (IdentityProviderMapperEntity entity : entities) { IdentityProviderMapperModel mapping = entityToModel(entity); mappings.add(mapping); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1508,23 +1535,26 @@ public class RealmAdapter implements RealmModel { @Override public Set getUserFederationMappers() { - Set mappers = new HashSet(); - for (UserFederationMapperEntity entity : this.realm.getUserFederationMappers()) { + Collection entities = this.realm.getUserFederationMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; + Set mappers = new HashSet<>(); + for (UserFederationMapperEntity entity : entities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override public Set getUserFederationMappersByFederationProvider(String federationProviderId) { - Set mappers = new HashSet(); Set mapperEntities = getUserFederationMapperEntitiesByFederationProvider(federationProviderId); + if (mapperEntities.isEmpty()) return Collections.EMPTY_SET; + Set mappers = new HashSet(); for (UserFederationMapperEntity entity : mapperEntities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -1693,13 +1723,13 @@ public class RealmAdapter implements RealmModel { TypedQuery query = em.createNamedQuery("getAuthenticationFlowsByRealm", AuthenticationFlowEntity.class); query.setParameter("realm", realm); List flows = query.getResultList(); - if (flows.size() == 0) return Collections.EMPTY_LIST; + if (flows.isEmpty()) return Collections.EMPTY_LIST; List models = new LinkedList<>(); for (AuthenticationFlowEntity entity : flows) { AuthenticationFlowModel model = entityToModel(entity); models.add(model); } - return models; + return Collections.unmodifiableList(models); } @Override @@ -1785,13 +1815,14 @@ public class RealmAdapter implements RealmModel { query.setParameter("realm", realm); query.setParameter("parentFlow", flow); List queryResult = query.getResultList(); + if (queryResult.isEmpty()) return Collections.EMPTY_LIST; List executions = new LinkedList<>(); for (AuthenticationExecutionEntity entity : queryResult) { AuthenticationExecutionModel model = entityToModel(entity); executions.add(model); } Collections.sort(executions, AuthenticationExecutionModel.ExecutionComparator.SINGLETON); - return executions; + return Collections.unmodifiableList(executions); } public AuthenticationExecutionModel entityToModel(AuthenticationExecutionEntity entity) { @@ -1916,11 +1947,13 @@ public class RealmAdapter implements RealmModel { @Override public List getAuthenticatorConfigs() { + Collection entities = realm.getAuthenticatorConfigs(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List authenticators = new LinkedList<>(); - for (AuthenticatorConfigEntity entity : realm.getAuthenticatorConfigs()) { + for (AuthenticatorConfigEntity entity : entities) { authenticators.add(entityToModel(entity)); } - return authenticators; + return Collections.unmodifiableList(authenticators); } @Override @@ -1993,11 +2026,13 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredActionProviders() { + Collection entities = realm.getRequiredActionProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List actions = new LinkedList<>(); - for (RequiredActionProviderEntity entity : realm.getRequiredActionProviders()) { + for (RequiredActionProviderEntity entity : entities) { actions.add(entityToModel(entity)); } - return actions; + return Collections.unmodifiableList(actions); } @Override @@ -2028,26 +2063,26 @@ public class RealmAdapter implements RealmModel { @Override public List getGroups() { + List groups = em.createNamedQuery("getAllGroupsByRealm").setParameter("realm", realm).getResultList(); + if (groups == null) return Collections.EMPTY_LIST; List list = new LinkedList<>(); - Collection groups = em.createNamedQuery("getAllGroupsByRealm").setParameter("realm", realm).getResultList(); - if (groups == null) return list; for (GroupEntity entity : groups) { list.add(new GroupAdapter(this, em, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParent() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -2100,12 +2135,13 @@ public class RealmAdapter implements RealmModel { @Override public List getClientTemplates() { + Collection entities = realm.getClientTemplates(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List list = new LinkedList<>(); - if (realm.getClientTemplates() == null) return list; - for (ClientTemplateEntity entity : realm.getClientTemplates()) { + for (ClientTemplateEntity entity : entities) { list.add(new ClientTemplateAdapter(this, em, session, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override 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 701cced0ea..9ceec417a6 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 @@ -645,14 +645,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .get(); List roles = getMongoStore().loadEntities(MongoRoleEntity.class, query, invocationContext); - Set result = new HashSet(); - if (roles == null) return result; + if (roles == null) return Collections.EMPTY_SET; + Set result = new HashSet(); for (MongoRoleEntity role : roles) { result.add(new RoleAdapter(session, this, role, this, invocationContext)); } - return result; + return Collections.unmodifiableSet(result); } @Override @@ -709,6 +709,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .and("realmId").is(getId()) .get(); List groups = getMongoStore().loadEntities(MongoGroupEntity.class, query, invocationContext); + if (groups == null) return Collections.EMPTY_LIST; List result = new LinkedList<>(); @@ -717,20 +718,20 @@ public class RealmAdapter extends AbstractMongoAdapter impleme result.add(model.getGroupById(group.getId(), this)); } - return result; + return Collections.unmodifiableList(result); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParentId() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -750,7 +751,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getDefaultRoles() { - return realm.getDefaultRoles(); + return Collections.unmodifiableList(realm.getDefaultRoles()); } @Override @@ -781,11 +782,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getDefaultGroups() { + List entities = realm.getDefaultGroups(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List defaultGroups = new LinkedList<>(); - for (String id : realm.getDefaultGroups()) { + for (String id : entities) { defaultGroups.add(session.realms().getGroupById(id, this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @Override @@ -812,11 +815,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getClientNameMap() { + List clients = getClients(); + if (clients.isEmpty()) return Collections.EMPTY_MAP; Map resourceMap = new HashMap(); - for (ClientModel resource : getClients()) { + for (ClientModel resource : clients) { resourceMap.put(resource.getClientId(), resource); } - return resourceMap; + return Collections.unmodifiableMap(resourceMap); } @Override @@ -826,11 +831,12 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .get(); List clientEntities = getMongoStore().loadEntities(MongoClientEntity.class, query, invocationContext); + if (clientEntities.isEmpty()) return Collections.EMPTY_LIST; List result = new ArrayList(); for (MongoClientEntity clientEntity : clientEntities) { result.add(new ClientAdapter(session, this, clientEntity, invocationContext)); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -922,8 +928,8 @@ public class RealmAdapter extends AbstractMongoAdapter impleme } protected List convertRequiredCredentialEntities(Collection credEntities) { - - List result = new ArrayList(); + if (credEntities == null || credEntities.isEmpty()) return Collections.EMPTY_LIST; + List result = new LinkedList<>(); for (RequiredCredentialEntity entity : credEntities) { RequiredCredentialModel model = new RequiredCredentialModel(); model.setFormLabel(entity.getFormLabel()); @@ -933,7 +939,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme result.add(model); } - return result; + return Collections.unmodifiableList(result); } protected void updateRealm() { @@ -950,7 +956,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getBrowserSecurityHeaders() { - return realm.getBrowserSecurityHeaders(); + return Collections.unmodifiableMap(realm.getBrowserSecurityHeaders()); } @Override @@ -961,7 +967,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getSmtpConfig() { - return realm.getSmtpConfig(); + return Collections.unmodifiableMap(realm.getSmtpConfig()); } @Override @@ -973,15 +979,20 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getIdentityProviders() { + List entities = realm.getIdentityProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List identityProviders = new ArrayList(); - for (IdentityProviderEntity entity: realm.getIdentityProviders()) { + for (IdentityProviderEntity entity: entities) { IdentityProviderModel identityProviderModel = new IdentityProviderModel(); identityProviderModel.setProviderId(entity.getProviderId()); identityProviderModel.setAlias(entity.getAlias()); identityProviderModel.setInternalId(entity.getInternalId()); - identityProviderModel.setConfig(entity.getConfig()); + Map config = entity.getConfig(); + Map copy = new HashMap<>(); + copy.putAll(config); + identityProviderModel.setConfig(copy); identityProviderModel.setEnabled(entity.isEnabled()); identityProviderModel.setTrustEmail(entity.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(entity.isAuthenticateByDefault()); @@ -993,7 +1004,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme identityProviders.add(identityProviderModel); } - return identityProviders; + return Collections.unmodifiableList(identityProviders); } @Override @@ -1132,6 +1143,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getUserFederationProviders() { List entities = realm.getUserFederationProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List copy = new LinkedList(); for (UserFederationProviderEntity entity : entities) { copy.add(entity); @@ -1151,7 +1163,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -1257,7 +1269,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getEventsListeners() { - return new HashSet(realm.getEventsListeners()); + List eventsListeners = realm.getEventsListeners(); + if (eventsListeners.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(eventsListeners); + return Collections.unmodifiableSet(copy); } @Override @@ -1272,7 +1288,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getEnabledEventTypes() { - return new HashSet(realm.getEnabledEventTypes()); + List enabledEventTypes = realm.getEnabledEventTypes(); + if (enabledEventTypes.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(enabledEventTypes); + return Collections.unmodifiableSet(copy); } @Override @@ -1364,7 +1384,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getSupportedLocales() { - return new HashSet(realm.getSupportedLocales()); + List supportedLocales = realm.getSupportedLocales(); + if (supportedLocales == null || supportedLocales.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(supportedLocales); + return Collections.unmodifiableSet(copy); } @Override @@ -1390,12 +1414,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getIdentityProviderMappers() { + List entities = getMongoEntity().getIdentityProviderMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappings = new HashSet(); - for (IdentityProviderMapperEntity entity : getMongoEntity().getIdentityProviderMappers()) { + for (IdentityProviderMapperEntity entity : entities) { IdentityProviderMapperModel mapping = entityToModel(entity); mappings.add(mapping); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1568,12 +1594,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getAuthenticationFlows() { List flows = getMongoEntity().getAuthenticationFlows(); + if (flows.isEmpty()) return Collections.EMPTY_LIST; List models = new LinkedList<>(); for (AuthenticationFlowEntity entity : flows) { AuthenticationFlowModel model = entityToModel(entity); models.add(model); } - return models; + return Collections.unmodifiableList(models); } @Override @@ -1662,7 +1689,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme executions.add(model); } Collections.sort(executions, AuthenticationExecutionModel.ExecutionComparator.SINGLETON); - return executions; + return Collections.unmodifiableList(executions); } public AuthenticationExecutionModel entityToModel(AuthenticationExecutionEntity entity) { @@ -1752,11 +1779,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getAuthenticatorConfigs() { + List entities = getMongoEntity().getAuthenticatorConfigs(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List authenticators = new LinkedList<>(); - for (AuthenticatorConfigEntity entity : getMongoEntity().getAuthenticatorConfigs()) { + for (AuthenticatorConfigEntity entity : entities) { authenticators.add(entityToModel(entity)); } - return authenticators; + return Collections.unmodifiableList(authenticators); } @Override @@ -1898,11 +1927,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getRequiredActionProviders() { + List entities = realm.getRequiredActionProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List actions = new LinkedList<>(); - for (RequiredActionProviderEntity entity : realm.getRequiredActionProviders()) { + for (RequiredActionProviderEntity entity : entities) { actions.add(entityToModel(entity)); } - return actions; + return Collections.unmodifiableList(actions); } public RequiredActionProviderEntity getRequiredActionProviderEntity(String id) { @@ -1930,12 +1961,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getUserFederationMappers() { + List entities = getMongoEntity().getUserFederationMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappers = new HashSet(); - for (UserFederationMapperEntity entity : getMongoEntity().getUserFederationMappers()) { + for (UserFederationMapperEntity entity : entities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -2053,12 +2086,12 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .and("realmId").is(getId()) .get(); List clientEntities = getMongoStore().loadEntities(MongoClientTemplateEntity.class, query, invocationContext); - + if (clientEntities.isEmpty()) return Collections.EMPTY_LIST; List result = new LinkedList<>(); for (MongoClientTemplateEntity clientEntity : clientEntities) { result.add(new ClientTemplateAdapter(session, this, clientEntity, invocationContext)); } - return result; + return Collections.unmodifiableList(result); } @Override diff --git a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java index 29b93318bd..a34cd9d29a 100755 --- a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java +++ b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java @@ -44,6 +44,7 @@ import java.security.PublicKey; import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -107,16 +108,19 @@ public class CachedRealm implements Serializable { protected String emailTheme; protected String masterAdminClient; - protected List requiredCredentials = new ArrayList(); - protected List userFederationProviders = new ArrayList(); + protected List requiredCredentials; + protected List userFederationProviders; protected MultivaluedHashMap userFederationMappers = new MultivaluedHashMap(); - protected List identityProviders = new ArrayList(); + protected Set userFederationMapperSet; + protected List identityProviders; - protected Map browserSecurityHeaders = new HashMap(); - protected Map smtpConfig = new HashMap(); + protected Map browserSecurityHeaders; + protected Map smtpConfig; protected Map authenticationFlows = new HashMap<>(); + protected List authenticationFlowList; protected Map authenticatorConfigs = new HashMap<>(); protected Map requiredActionProviders = new HashMap<>(); + protected List requiredActionProviderList; protected Map requiredActionProvidersByAlias = new HashMap<>(); protected MultivaluedHashMap authenticationExecutions = new MultivaluedHashMap<>(); protected Map executionsById = new HashMap<>(); @@ -129,21 +133,27 @@ public class CachedRealm implements Serializable { protected boolean eventsEnabled; protected long eventsExpiration; - protected Set eventsListeners = new HashSet(); - protected Set enabledEventTypes = new HashSet(); + protected Set eventsListeners; + protected Set enabledEventTypes; protected boolean adminEventsEnabled; protected Set adminEnabledEventOperations = new HashSet(); protected boolean adminEventsDetailsEnabled; - protected List defaultRoles = new LinkedList(); + protected List defaultRoles; + + public Set getIdentityProviderMapperSet() { + return identityProviderMapperSet; + } + protected List defaultGroups = new LinkedList(); protected Set groups = new HashSet(); protected Map realmRoles = new HashMap(); protected Map clients = new HashMap(); protected List clientTemplates= new LinkedList<>(); protected boolean internationalizationEnabled; - protected Set supportedLocales = new HashSet(); + protected Set supportedLocales; protected String defaultLocale; protected MultivaluedHashMap identityProviderMappers = new MultivaluedHashMap<>(); + protected Set identityProviderMapperSet; public CachedRealm() { } @@ -200,8 +210,9 @@ public class CachedRealm implements Serializable { requiredCredentials = model.getRequiredCredentials(); userFederationProviders = model.getUserFederationProviders(); - for (UserFederationMapperModel mapper : model.getUserFederationMappers()) { - userFederationMappers.add(mapper.getFederationProviderId(), mapper); + userFederationMapperSet = model.getUserFederationMappers(); + for (UserFederationMapperModel mapper : userFederationMapperSet) { + this.userFederationMappers.add(mapper.getFederationProviderId(), mapper); } this.identityProviders = new ArrayList<>(); @@ -209,6 +220,7 @@ public class CachedRealm implements Serializable { for (IdentityProviderModel identityProviderModel : model.getIdentityProviders()) { this.identityProviders.add(new IdentityProviderModel(identityProviderModel)); } + this.identityProviders = Collections.unmodifiableList(this.identityProviders); for (IdentityProviderMapperModel mapper : model.getIdentityProviderMappers()) { identityProviderMappers.add(mapper.getIdentityProviderAlias(), mapper); @@ -216,18 +228,18 @@ public class CachedRealm implements Serializable { - smtpConfig.putAll(model.getSmtpConfig()); - browserSecurityHeaders.putAll(model.getBrowserSecurityHeaders()); + smtpConfig = model.getSmtpConfig(); + browserSecurityHeaders = model.getBrowserSecurityHeaders(); eventsEnabled = model.isEventsEnabled(); eventsExpiration = model.getEventsExpiration(); - eventsListeners.addAll(model.getEventsListeners()); - enabledEventTypes.addAll(model.getEnabledEventTypes()); + eventsListeners = model.getEventsListeners(); + enabledEventTypes = model.getEnabledEventTypes(); adminEventsEnabled = model.isAdminEventsEnabled(); adminEventsDetailsEnabled = model.isAdminEventsDetailsEnabled(); - defaultRoles.addAll(model.getDefaultRoles()); + defaultRoles = model.getDefaultRoles(); ClientModel masterAdminClient = model.getMasterAdminClient(); this.masterAdminClient = (masterAdminClient != null) ? masterAdminClient.getId() : null; @@ -238,10 +250,11 @@ public class CachedRealm implements Serializable { cacheClientTemplates(cache, delegate, model); internationalizationEnabled = model.isInternationalizationEnabled(); - supportedLocales.addAll(model.getSupportedLocales()); + supportedLocales = model.getSupportedLocales(); defaultLocale = model.getDefaultLocale(); - for (AuthenticationFlowModel flow : model.getAuthenticationFlows()) { - authenticationFlows.put(flow.getId(), flow); + authenticationFlowList = model.getAuthenticationFlows(); + for (AuthenticationFlowModel flow : authenticationFlowList) { + this.authenticationFlows.put(flow.getId(), flow); authenticationExecutions.put(flow.getId(), new LinkedList()); for (AuthenticationExecutionModel execution : model.getAuthenticationExecutions(flow.getId())) { authenticationExecutions.add(flow.getId(), execution); @@ -254,8 +267,9 @@ public class CachedRealm implements Serializable { for (AuthenticatorConfigModel authenticator : model.getAuthenticatorConfigs()) { authenticatorConfigs.put(authenticator.getId(), authenticator); } - for (RequiredActionProviderModel action : model.getRequiredActionProviders()) { - requiredActionProviders.put(action.getId(), action); + requiredActionProviderList = model.getRequiredActionProviders(); + for (RequiredActionProviderModel action : requiredActionProviderList) { + this.requiredActionProviders.put(action.getId(), action); requiredActionProvidersByAlias.put(action.getAlias(), action); } @@ -606,4 +620,16 @@ public class CachedRealm implements Serializable { public X509Certificate getCertificate() { return certificate; } + + public Set getUserFederationMapperSet() { + return userFederationMapperSet; + } + + public List getAuthenticationFlowList() { + return authenticationFlowList; + } + + public List getRequiredActionProviderList() { + return requiredActionProviderList; + } } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 8b59053102..bc8b0b6e7c 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -400,6 +400,9 @@ public class ModelToRepresentation { rep.setRequiredActions(new LinkedList()); List requiredActionProviders = realm.getRequiredActionProviders(); + List copy = new LinkedList<>(); + copy.addAll(requiredActionProviders); + requiredActionProviders = copy; //ensure consistent ordering of requiredActionProviders. Collections.sort(requiredActionProviders, new Comparator() { @Override diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index eea67920d9..921e682e24 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -46,6 +46,8 @@ import javax.mail.internet.MimeMessage; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.*; @@ -542,8 +544,11 @@ public class ResetPasswordTest { keycloakRule.configure(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - host[0] = appRealm.getSmtpConfig().get("host"); - appRealm.getSmtpConfig().put("host", "invalid_host"); + Map smtpConfig = new HashMap<>(); + smtpConfig.putAll(appRealm.getSmtpConfig()); + host[0] = smtpConfig.get("host"); + smtpConfig.put("host", "invalid_host"); + appRealm.setSmtpConfig(smtpConfig); } }); @@ -568,7 +573,10 @@ public class ResetPasswordTest { keycloakRule.configure(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getSmtpConfig().put("host",host[0]); + Map smtpConfig = new HashMap<>(); + smtpConfig.putAll(appRealm.getSmtpConfig()); + smtpConfig.put("host",host[0]); + appRealm.setSmtpConfig(smtpConfig); } }); }