From 11ff26699d72d1d9bf939dc0503a4b0dab5cab32 Mon Sep 17 00:00:00 2001 From: vramik Date: Mon, 10 Jun 2024 15:32:48 +0200 Subject: [PATCH 1/4] Provide a cache layer for the organization model Closes #30087 Signed-off-by: vramik --- .../cache/infinispan/RealmCacheSession.java | 23 ++ .../organization/CachedOrganization.java | 87 +++++++ .../InfinispanOrganizationProvider.java | 222 ++++++++++++++++++ ...InfinispanOrganizationProviderFactory.java | 76 ++++++ .../organization/OrganizationAdapter.java | 148 ++++++++++++ ...k.organization.OrganizationProviderFactory | 18 ++ .../models/cache/CacheRealmProvider.java | 1 + .../organization/OrganizationProvider.java | 4 +- .../admin/resource/OrganizationsResource.java | 4 - .../AbstractBrokerSelfRegistrationTest.java | 5 + .../OrganizationIdentityProviderTest.java | 13 +- 11 files changed, 593 insertions(+), 8 deletions(-) create mode 100644 model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java create mode 100644 model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java create mode 100644 model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java create mode 100644 model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/OrganizationAdapter.java create mode 100644 model/infinispan/src/main/resources/META-INF/services/org.keycloak.organization.OrganizationProviderFactory rename {server-spi-private => server-spi}/src/main/java/org/keycloak/organization/OrganizationProvider.java (97%) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java index c8c672258f02..63c615eaa801 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java @@ -172,6 +172,29 @@ public GroupProvider getGroupDelegate() { return groupDelegate; } + public Set getInvalidations() { + return Collections.unmodifiableSet(invalidations); + } + + public RealmCacheManager getCache() { + return cache; + } + + public long getStartupRevision() { + return startupRevision; + } + + @Override + public void registerInvalidation(String id) { + invalidations.add(id); + invalidationEvents.add(new InvalidationEvent() { + @Override + public String getId() { + return id; + } + }); + } + @Override public void registerRealmInvalidation(String id, String name) { cache.realmUpdated(id, name, invalidations); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java new file mode 100644 index 000000000000..000c6e9484e4 --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java @@ -0,0 +1,87 @@ +/* + * Copyright 2024 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.models.cache.infinispan.organization; + +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.OrganizationDomainModel; +import org.keycloak.models.OrganizationModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.cache.infinispan.DefaultLazyLoader; +import org.keycloak.models.cache.infinispan.LazyLoader; +import org.keycloak.models.cache.infinispan.entities.AbstractRevisioned; +import org.keycloak.models.cache.infinispan.entities.InRealm; + +public class CachedOrganization extends AbstractRevisioned implements InRealm { + + private final RealmModel realm; + private final String name; + private final String description; + private final boolean enabled; + private final LazyLoader> attributes; + private final Set domains; + private final Set idps; + + public CachedOrganization(Long revision, RealmModel realm, OrganizationModel organization) { + super(revision, organization.getId()); + this.realm = realm; + this.name = organization.getName(); + this.description = organization.getDescription(); + this.enabled = organization.isEnabled(); + this.attributes = new DefaultLazyLoader<>(orgModel -> new MultivaluedHashMap<>(orgModel.getAttributes()), MultivaluedHashMap::new); + this.domains = organization.getDomains().collect(Collectors.toSet()); + this.idps = organization.getIdentityProviders().collect(Collectors.toSet()); + } + + @Override + public String getRealm() { + return realm.getId(); + } + + public RealmModel getRealmModel() { + return realm; + } + + public String getName() { + return name; + } + + public String getDescription() { + return description; + } + + public boolean isEnabled() { + return enabled; + } + + public MultivaluedHashMap getAttributes(Supplier organizationModel) { + return attributes.get(organizationModel); + } + + public Stream getDomains() { + return domains.stream(); + } + + public Stream getIdentityProviders() { + return idps.stream(); + } +} diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java new file mode 100644 index 000000000000..29a622e4bf3e --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java @@ -0,0 +1,222 @@ +/* + * Copyright 2024 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.models.cache.infinispan.organization; + +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.OrganizationModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.cache.CacheRealmProvider; +import org.keycloak.models.cache.infinispan.RealmCacheSession; +import org.keycloak.organization.OrganizationProvider; + +public class InfinispanOrganizationProvider implements OrganizationProvider { + + private final KeycloakSession session; + private final OrganizationProvider jpaOrgDelegate; + private final RealmCacheSession realmCache; + private final Map managedOrganizations = new HashMap<>(); + + public InfinispanOrganizationProvider(KeycloakSession session) { + this.session = session; + this.jpaOrgDelegate = session.getProvider(OrganizationProvider.class, "jpa"); + this.realmCache = (RealmCacheSession) session.getProvider(CacheRealmProvider.class); + } + + @Override + public OrganizationModel create(String name) { + return jpaOrgDelegate.create(name); + } + + @Override + public OrganizationModel getById(String id) { + CachedOrganization cached = realmCache.getCache().get(id, CachedOrganization.class); + String realmId = getRealm().getId(); + if (cached != null && !cached.getRealm().equals(realmId)) { + cached = null; + } + + if (cached == null) { + Long loaded = realmCache.getCache().getCurrentRevision(id); + OrganizationModel model = jpaOrgDelegate.getById(id); + if (model == null) return null; + if (realmCache.getInvalidations().contains(id)) return model; + cached = new CachedOrganization(loaded, getRealm(), model); + realmCache.getCache().addRevisioned(cached, realmCache.getStartupRevision()); + + // no need to check for realm invalidation as IdP changes are handled by events within InfinispanOrganizationProviderFactory + } else if (realmCache.getInvalidations().contains(id)) { + return orgDelegate.getById(id); + } else if (managedOrganizations.containsKey(id)) { + return managedOrganizations.get(id); + } + OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, jpaOrgDelegate); + managedOrganizations.put(id, adapter); + return adapter; + } + + @Override + public OrganizationModel getByDomainName(String domainName) { + String cacheKey = getRealm().getId() + "+.org.domain.name." + domainName; + CachedOrganization cached = realmCache.getCache().get(cacheKey, CachedOrganization.class); + String realmId = getRealm().getId(); + if (cached != null && !cached.getRealm().equals(realmId)) { + cached = null; + } + + if (cached == null) { + Long loaded = realmCache.getCache().getCurrentRevision(cacheKey); + OrganizationModel model = jpaOrgDelegate.getByDomainName(domainName); + if (model == null) return null; + if (realmCache.getInvalidations().contains(model.getId())) return model; + cached = new CachedOrganization(loaded, getRealm(), model); + realmCache.getCache().addRevisioned(cached, realmCache.getStartupRevision()); + + // no need to check for realm invalidation as IdP changes are handled by events within InfinispanOrganizationProviderFactory + } else if (realmCache.getInvalidations().contains(cached.getId())) { + return orgDelegate.getByDomainName(domainName); + } else if (managedOrganizations.containsKey(cached.getId())) { + return managedOrganizations.get(cached.getId()); + } + OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, jpaOrgDelegate); + managedOrganizations.put(cacheKey, adapter); + return adapter; + } + + @Override + public Stream getAllStream(String search, Boolean exact, Integer first, Integer max) { + // Return cache delegates to ensure cache invalidation during write operations + return getCacheDelegates(jpaOrgDelegate.getAllStream(search, exact, first, max)); + } + + @Override + public Stream getAllStream(Map attributes, Integer first, Integer max) { + // Return cache delegates to ensure cache invalidation during write operations + return getCacheDelegates(jpaOrgDelegate.getAllStream(attributes, first, max)); + } + + @Override + public boolean remove(OrganizationModel organization) { + registerOrganizationInvalidation(organization.getId()); + return jpaOrgDelegate.remove(organization); + } + + @Override + public void removeAll() { + //TODO: won't scale, requires a better mechanism for bulk deleting organizations within a realm + //this way, all organizations in the realm will be invalidated ... or should it be invalidated whole realm instead? + getAllStream().forEach(this::remove); + } + + @Override + public boolean addMember(OrganizationModel organization, UserModel user) { + return jpaOrgDelegate.addMember(organization, user); + } + + @Override + public boolean removeMember(OrganizationModel organization, UserModel member) { + return jpaOrgDelegate.removeMember(organization, member); + } + + @Override + public Stream getMembersStream(OrganizationModel organization, String search, Boolean exact, Integer first, Integer max) { + return jpaOrgDelegate.getMembersStream(organization, search, exact, first, max); + } + + @Override + public UserModel getMemberById(OrganizationModel organization, String id) { + return jpaOrgDelegate.getMemberById(organization, id); + } + + @Override + public OrganizationModel getByMember(UserModel member) { + // Return cache delegate to ensure cache invalidation during write operations + return getCacheDelegate(jpaOrgDelegate.getByMember(member)); + } + + @Override + public boolean isManagedMember(OrganizationModel organization, UserModel member) { + return jpaOrgDelegate.isManagedMember(organization, member); + } + + @Override + public boolean addIdentityProvider(OrganizationModel organization, IdentityProviderModel identityProvider) { + boolean added = jpaOrgDelegate.addIdentityProvider(organization, identityProvider); + if (added) { + registerOrganizationInvalidation(organization.getId()); + } + return added; + } + + @Override + public Stream getIdentityProviders(OrganizationModel organization) { + return jpaOrgDelegate.getIdentityProviders(organization); + } + + @Override + public boolean removeIdentityProvider(OrganizationModel organization, IdentityProviderModel identityProvider) { + boolean removed = jpaOrgDelegate.removeIdentityProvider(organization, identityProvider); + if (removed) { + registerOrganizationInvalidation(organization.getId()); + } + return removed; + } + + @Override + public boolean isEnabled() { + return getRealm().isOrganizationsEnabled(); + } + + @Override + public long count() { + return jpaOrgDelegate.count(); + } + + @Override + public void close() { + jpaOrgDelegate.close(); + } + + void registerOrganizationInvalidation(String orgId) { + OrganizationAdapter adapter = managedOrganizations.get(orgId); + if (adapter != null) { + adapter.invalidate(); + } + + realmCache.registerInvalidation(orgId); + } + + private RealmModel getRealm() { + RealmModel realm = session.getContext().getRealm(); + if (realm == null) { + throw new IllegalArgumentException("Session not bound to a realm"); + } + return realm; + } + + private OrganizationModel getCacheDelegate(OrganizationModel model) { + return model == null ? null : getById(model.getId()); + } + + private Stream getCacheDelegates(Stream backendOrganizations) { + return backendOrganizations.map(this::getCacheDelegate); + } +} diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java new file mode 100644 index 000000000000..7293c100092f --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProviderFactory.java @@ -0,0 +1,76 @@ +/* + * Copyright 2024 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.models.cache.infinispan.organization; + +import org.keycloak.Config.Scope; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.models.RealmModel; +import org.keycloak.organization.OrganizationProvider; +import org.keycloak.models.OrganizationModel; +import org.keycloak.organization.OrganizationProviderFactory; + +public class InfinispanOrganizationProviderFactory implements OrganizationProviderFactory { + + public static final String PROVIDER_ID = "infinispan"; + + @Override + public OrganizationProvider create(KeycloakSession session) { + return new InfinispanOrganizationProvider(session); + } + + @Override + public void init(Scope config) { + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + factory.register(event -> { + if (event instanceof RealmModel.IdentityProviderUpdatedEvent idpUpdatedEvent) { + registerOrganizationInvalidation(idpUpdatedEvent.getKeycloakSession(), idpUpdatedEvent.getUpdatedIdentityProvider()); + } + if (event instanceof RealmModel.IdentityProviderRemovedEvent idpRemovedEvent) { + registerOrganizationInvalidation(idpRemovedEvent.getKeycloakSession(), idpRemovedEvent.getRemovedIdentityProvider()); + } + }); + } + + private void registerOrganizationInvalidation(KeycloakSession session, IdentityProviderModel idp) { + if (idp.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE) != null) { + InfinispanOrganizationProvider orgProvider = (InfinispanOrganizationProvider) session.getProvider(OrganizationProvider.class, getId()); + if (orgProvider != null) { + orgProvider.registerOrganizationInvalidation(idp.getConfig().get(OrganizationModel.ORGANIZATION_ATTRIBUTE)); + } + } + } + + @Override + public void close() { + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public int order() { + return 10; + } +} diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/OrganizationAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/OrganizationAdapter.java new file mode 100644 index 000000000000..b4865991812d --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/OrganizationAdapter.java @@ -0,0 +1,148 @@ +/* + * Copyright 2024 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.models.cache.infinispan.organization; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Stream; +import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.OrganizationDomainModel; +import org.keycloak.models.OrganizationModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.cache.CacheRealmProvider; +import org.keycloak.organization.OrganizationProvider; + +public class OrganizationAdapter implements OrganizationModel { + + private volatile boolean invalidated; + private volatile OrganizationModel updated; + private final Supplier modelSupplier; + private final CacheRealmProvider realmCache; + private final CachedOrganization cached; + private final OrganizationProvider delegate; + + public OrganizationAdapter(CachedOrganization cached, CacheRealmProvider realmCache, OrganizationProvider delegate) { + this.cached = cached; + this.realmCache = realmCache; + this.delegate = delegate; + this.modelSupplier = this::getOrganizationModel; + } + + void invalidate() { + invalidated = true; + } + + private OrganizationModel getOrganizationModel() { + return delegate.getById(cached.getId()); + } + + private boolean isUpdated() { + if (updated != null) return true; + if (!invalidated) return false; + updated = getOrganizationModel(); + if (updated == null) throw new IllegalStateException("Not found in database"); + return true; + } + + private void getDelegateForUpdate() { + if (updated == null) { + realmCache.registerInvalidation(cached.getId()); + updated = modelSupplier.get(); + if (updated == null) throw new IllegalStateException("Not found in database"); + } + } + + @Override + public String getId() { + if (isUpdated()) return updated.getId(); + return cached.getId(); + } + + @Override + public String getName() { + if (isUpdated()) return updated.getName() ; + return cached.getName(); + } + + @Override + public void setName(String name) { + getDelegateForUpdate(); + updated.setName(name); + } + + @Override + public boolean isEnabled() { + if (isUpdated()) return updated.isEnabled(); + return cached.isEnabled(); + } + + @Override + public void setEnabled(boolean enabled) { + getDelegateForUpdate(); + updated.setEnabled(enabled); + } + + @Override + public String getDescription() { + if (isUpdated()) return updated.getDescription(); + return cached.getDescription(); + } + + @Override + public void setDescription(String description) { + getDelegateForUpdate(); + updated.setDescription(description); + } + + @Override + public Map> getAttributes() { + if (isUpdated()) return updated.getAttributes(); + return cached.getAttributes(modelSupplier); + } + + @Override + public void setAttributes(Map> attributes) { + getDelegateForUpdate(); + updated.setAttributes(attributes); + } + + @Override + public Stream getDomains() { + if (isUpdated()) return updated.getDomains(); + return cached.getDomains(); + } + + @Override + public void setDomains(Set domains) { + getDelegateForUpdate(); + updated.setDomains(domains); + } + + @Override + public Stream getIdentityProviders() { + if (isUpdated()) return updated.getIdentityProviders(); + return cached.getIdentityProviders(); + } + + @Override + public boolean isManaged(UserModel user) { + return delegate.isManagedMember(this, user); + } + +} diff --git a/model/infinispan/src/main/resources/META-INF/services/org.keycloak.organization.OrganizationProviderFactory b/model/infinispan/src/main/resources/META-INF/services/org.keycloak.organization.OrganizationProviderFactory new file mode 100644 index 000000000000..763e4badcf94 --- /dev/null +++ b/model/infinispan/src/main/resources/META-INF/services/org.keycloak.organization.OrganizationProviderFactory @@ -0,0 +1,18 @@ +# +# Copyright 2024 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. +# + +org.keycloak.models.cache.infinispan.organization.InfinispanOrganizationProviderFactory diff --git a/model/storage-private/src/main/java/org/keycloak/models/cache/CacheRealmProvider.java b/model/storage-private/src/main/java/org/keycloak/models/cache/CacheRealmProvider.java index 6215cf67f8fc..c45257898f13 100755 --- a/model/storage-private/src/main/java/org/keycloak/models/cache/CacheRealmProvider.java +++ b/model/storage-private/src/main/java/org/keycloak/models/cache/CacheRealmProvider.java @@ -39,4 +39,5 @@ public interface CacheRealmProvider extends RealmProvider, ClientProvider, Clien void registerRoleInvalidation(String id, String roleName, String roleContainerId); void registerGroupInvalidation(String id); + void registerInvalidation(String id); } diff --git a/server-spi-private/src/main/java/org/keycloak/organization/OrganizationProvider.java b/server-spi/src/main/java/org/keycloak/organization/OrganizationProvider.java similarity index 97% rename from server-spi-private/src/main/java/org/keycloak/organization/OrganizationProvider.java rename to server-spi/src/main/java/org/keycloak/organization/OrganizationProvider.java index 7fd84d889ea7..64cb01eac842 100644 --- a/server-spi-private/src/main/java/org/keycloak/organization/OrganizationProvider.java +++ b/server-spi/src/main/java/org/keycloak/organization/OrganizationProvider.java @@ -147,12 +147,12 @@ default Stream getAllStream() { /** * @param organization the organization - * @return The identityProvider associated with a given {@code organization} or {@code null} if there is none. + * @return Stream of the identity providers associated with the given {@code organization}. Never returns {@code null}. */ Stream getIdentityProviders(OrganizationModel organization); /** - * Removes the link between the given {@link OrganizationModel} and identity provider associated with it if such a link exists. + * Removes the link between the given {@link OrganizationModel} and the identity provider associated with it if such a link exists. * * @param organization the organization * @param identityProvider the identity provider diff --git a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java index a7b03e00bd03..1422fdac36b5 100644 --- a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java +++ b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java @@ -17,11 +17,8 @@ package org.keycloak.organization.admin.resource; -import static java.util.Optional.ofNullable; import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import jakarta.ws.rs.Consumes; @@ -45,7 +42,6 @@ import org.keycloak.models.OrganizationModel; import org.keycloak.organization.OrganizationProvider; import org.keycloak.organization.utils.Organizations; -import org.keycloak.representations.idm.OrganizationDomainRepresentation; import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.services.ErrorResponse; import org.keycloak.services.resources.KeycloakOpenAPI; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java index 78a3133e7b97..3c1a50dc339e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java @@ -17,6 +17,8 @@ package org.keycloak.testsuite.organization.broker; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -45,6 +47,7 @@ import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.organization.admin.AbstractOrganizationTest; +import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.util.UserBuilder; public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganizationTest { @@ -783,6 +786,8 @@ public void testMemberFromBrokerRedirectedToOriginBroker() { driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/")); log.debug("Updating info on updateAccount page"); updateAccountInformationPage.updateAccountInformation(user.getUsername(), user.getEmail(), "Firstname", "Lastname"); + assertThat(appPage.getRequestType(),is(AppPage.RequestType.AUTH_RESPONSE)); + UserRepresentation account = getUserRepresentation(user.getEmail()); realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); realmsResouce().realm(bc.providerRealmName()).logoutAll(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java index 2d0917a6a742..76fc3e347afb 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/OrganizationIdentityProviderTest.java @@ -111,11 +111,20 @@ public void testDelete() { IdentityProviderRepresentation idpTemplate = organization .identityProviders().get(bc.getIDPAlias()).toRepresentation(); + //remove Org related stuff from the template + idpTemplate.getConfig().remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + idpTemplate.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); + idpTemplate.getConfig().remove(OrganizationModel.IdentityProviderRedirectMode.EMAIL_MATCH.getKey()); + for (int i = 0; i < 5; i++) { idpTemplate.setAlias("idp-" + i); idpTemplate.setInternalId(null); - testRealm().identityProviders().create(idpTemplate).close(); - organization.identityProviders().addIdentityProvider(idpTemplate.getAlias()).close(); + try (Response response = testRealm().identityProviders().create(idpTemplate)) { + assertThat("Falied to create idp-" + i, response.getStatus(), equalTo(Status.CREATED.getStatusCode())); + } + try (Response response = organization.identityProviders().addIdentityProvider(idpTemplate.getAlias())) { + assertThat("Falied to add idp-" + i, response.getStatus(), equalTo(Status.NO_CONTENT.getStatusCode())); + } } Assert.assertEquals(6, organization.identityProviders().getIdentityProviders().size()); From 32f892fb9f6f28d3ca9910d0006dbc87d3e7aae9 Mon Sep 17 00:00:00 2001 From: vramik Date: Wed, 12 Jun 2024 15:59:08 +0200 Subject: [PATCH 2/4] rename jpaOrgDelegate to orgDelegate Signed-off-by: vramik --- .../InfinispanOrganizationProvider.java | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java index 29a622e4bf3e..74b748589781 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java @@ -31,19 +31,19 @@ public class InfinispanOrganizationProvider implements OrganizationProvider { private final KeycloakSession session; - private final OrganizationProvider jpaOrgDelegate; + private final OrganizationProvider orgDelegate; private final RealmCacheSession realmCache; private final Map managedOrganizations = new HashMap<>(); public InfinispanOrganizationProvider(KeycloakSession session) { this.session = session; - this.jpaOrgDelegate = session.getProvider(OrganizationProvider.class, "jpa"); + this.orgDelegate = session.getProvider(OrganizationProvider.class, "jpa"); this.realmCache = (RealmCacheSession) session.getProvider(CacheRealmProvider.class); } @Override public OrganizationModel create(String name) { - return jpaOrgDelegate.create(name); + return orgDelegate.create(name); } @Override @@ -56,7 +56,7 @@ public OrganizationModel getById(String id) { if (cached == null) { Long loaded = realmCache.getCache().getCurrentRevision(id); - OrganizationModel model = jpaOrgDelegate.getById(id); + OrganizationModel model = orgDelegate.getById(id); if (model == null) return null; if (realmCache.getInvalidations().contains(id)) return model; cached = new CachedOrganization(loaded, getRealm(), model); @@ -68,7 +68,7 @@ public OrganizationModel getById(String id) { } else if (managedOrganizations.containsKey(id)) { return managedOrganizations.get(id); } - OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, jpaOrgDelegate); + OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, orgDelegate); managedOrganizations.put(id, adapter); return adapter; } @@ -84,7 +84,7 @@ public OrganizationModel getByDomainName(String domainName) { if (cached == null) { Long loaded = realmCache.getCache().getCurrentRevision(cacheKey); - OrganizationModel model = jpaOrgDelegate.getByDomainName(domainName); + OrganizationModel model = orgDelegate.getByDomainName(domainName); if (model == null) return null; if (realmCache.getInvalidations().contains(model.getId())) return model; cached = new CachedOrganization(loaded, getRealm(), model); @@ -96,7 +96,7 @@ public OrganizationModel getByDomainName(String domainName) { } else if (managedOrganizations.containsKey(cached.getId())) { return managedOrganizations.get(cached.getId()); } - OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, jpaOrgDelegate); + OrganizationAdapter adapter = new OrganizationAdapter(cached, realmCache, orgDelegate); managedOrganizations.put(cacheKey, adapter); return adapter; } @@ -104,19 +104,19 @@ public OrganizationModel getByDomainName(String domainName) { @Override public Stream getAllStream(String search, Boolean exact, Integer first, Integer max) { // Return cache delegates to ensure cache invalidation during write operations - return getCacheDelegates(jpaOrgDelegate.getAllStream(search, exact, first, max)); + return getCacheDelegates(orgDelegate.getAllStream(search, exact, first, max)); } @Override public Stream getAllStream(Map attributes, Integer first, Integer max) { // Return cache delegates to ensure cache invalidation during write operations - return getCacheDelegates(jpaOrgDelegate.getAllStream(attributes, first, max)); + return getCacheDelegates(orgDelegate.getAllStream(attributes, first, max)); } @Override public boolean remove(OrganizationModel organization) { registerOrganizationInvalidation(organization.getId()); - return jpaOrgDelegate.remove(organization); + return orgDelegate.remove(organization); } @Override @@ -128,38 +128,38 @@ public void removeAll() { @Override public boolean addMember(OrganizationModel organization, UserModel user) { - return jpaOrgDelegate.addMember(organization, user); + return orgDelegate.addMember(organization, user); } @Override public boolean removeMember(OrganizationModel organization, UserModel member) { - return jpaOrgDelegate.removeMember(organization, member); + return orgDelegate.removeMember(organization, member); } @Override public Stream getMembersStream(OrganizationModel organization, String search, Boolean exact, Integer first, Integer max) { - return jpaOrgDelegate.getMembersStream(organization, search, exact, first, max); + return orgDelegate.getMembersStream(organization, search, exact, first, max); } @Override public UserModel getMemberById(OrganizationModel organization, String id) { - return jpaOrgDelegate.getMemberById(organization, id); + return orgDelegate.getMemberById(organization, id); } @Override public OrganizationModel getByMember(UserModel member) { // Return cache delegate to ensure cache invalidation during write operations - return getCacheDelegate(jpaOrgDelegate.getByMember(member)); + return getCacheDelegate(orgDelegate.getByMember(member)); } @Override public boolean isManagedMember(OrganizationModel organization, UserModel member) { - return jpaOrgDelegate.isManagedMember(organization, member); + return orgDelegate.isManagedMember(organization, member); } @Override public boolean addIdentityProvider(OrganizationModel organization, IdentityProviderModel identityProvider) { - boolean added = jpaOrgDelegate.addIdentityProvider(organization, identityProvider); + boolean added = orgDelegate.addIdentityProvider(organization, identityProvider); if (added) { registerOrganizationInvalidation(organization.getId()); } @@ -168,12 +168,12 @@ public boolean addIdentityProvider(OrganizationModel organization, IdentityProvi @Override public Stream getIdentityProviders(OrganizationModel organization) { - return jpaOrgDelegate.getIdentityProviders(organization); + return orgDelegate.getIdentityProviders(organization); } @Override public boolean removeIdentityProvider(OrganizationModel organization, IdentityProviderModel identityProvider) { - boolean removed = jpaOrgDelegate.removeIdentityProvider(organization, identityProvider); + boolean removed = orgDelegate.removeIdentityProvider(organization, identityProvider); if (removed) { registerOrganizationInvalidation(organization.getId()); } @@ -187,12 +187,12 @@ public boolean isEnabled() { @Override public long count() { - return jpaOrgDelegate.count(); + return orgDelegate.count(); } @Override public void close() { - jpaOrgDelegate.close(); + orgDelegate.close(); } void registerOrganizationInvalidation(String orgId) { From bfb96c46a88b45397f2ab34899a6a833597efc86 Mon Sep 17 00:00:00 2001 From: vramik Date: Wed, 12 Jun 2024 19:46:40 +0200 Subject: [PATCH 3/4] cache count Signed-off-by: vramik --- .../organization/CachedOrganization.java | 1 - .../organization/CachedOrganizationCount.java | 42 +++++++++++++++++++ .../InfinispanOrganizationProvider.java | 37 ++++++++++++---- .../organization/admin/OrganizationTest.java | 14 +++++-- 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganizationCount.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java index 000c6e9484e4..13a603aff6ca 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganization.java @@ -25,7 +25,6 @@ import org.keycloak.models.OrganizationDomainModel; import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; -import org.keycloak.models.UserModel; import org.keycloak.models.cache.infinispan.DefaultLazyLoader; import org.keycloak.models.cache.infinispan.LazyLoader; import org.keycloak.models.cache.infinispan.entities.AbstractRevisioned; diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganizationCount.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganizationCount.java new file mode 100644 index 000000000000..8d029369aa66 --- /dev/null +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/CachedOrganizationCount.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 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.models.cache.infinispan.organization; + +import org.keycloak.models.RealmModel; +import org.keycloak.models.cache.infinispan.entities.AbstractRevisioned; +import org.keycloak.models.cache.infinispan.entities.InRealm; + +public class CachedOrganizationCount extends AbstractRevisioned implements InRealm { + + private final RealmModel realm; + private final long count; + + public CachedOrganizationCount(Long revision, RealmModel realm, long count) { + super(revision, InfinispanOrganizationProvider.cacheKeyOrgCount(realm)); + this.realm = realm; + this.count = count; + } + + @Override + public String getRealm() { + return realm.getId(); + } + + public long getCount() { + return count; + } +} diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java index 74b748589781..4f08231b318a 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java @@ -41,11 +41,23 @@ public InfinispanOrganizationProvider(KeycloakSession session) { this.realmCache = (RealmCacheSession) session.getProvider(CacheRealmProvider.class); } + static String cacheKeyOrgCount(RealmModel realm) { + return realm.getId() + ".org.count"; + } + @Override public OrganizationModel create(String name) { + registerCountInvalidation(); return orgDelegate.create(name); } + @Override + public boolean remove(OrganizationModel organization) { + registerOrganizationInvalidation(organization.getId()); + registerCountInvalidation(); + return orgDelegate.remove(organization); + } + @Override public OrganizationModel getById(String id) { CachedOrganization cached = realmCache.getCache().get(id, CachedOrganization.class); @@ -113,12 +125,6 @@ public Stream getAllStream(Map attributes, In return getCacheDelegates(orgDelegate.getAllStream(attributes, first, max)); } - @Override - public boolean remove(OrganizationModel organization) { - registerOrganizationInvalidation(organization.getId()); - return orgDelegate.remove(organization); - } - @Override public void removeAll() { //TODO: won't scale, requires a better mechanism for bulk deleting organizations within a realm @@ -187,7 +193,20 @@ public boolean isEnabled() { @Override public long count() { - return orgDelegate.count(); + String cacheKey = cacheKeyOrgCount(getRealm()); + CachedOrganizationCount cached = realmCache.getCache().get(cacheKey, CachedOrganizationCount.class); + + // cached and not invalidated + if (cached != null && !realmCache.getInvalidations().contains(cacheKey)) { + return cached.getCount(); + } + + Long loaded = realmCache.getCache().getCurrentRevision(cacheKey); + long count = orgDelegate.count(); + cached = new CachedOrganizationCount(loaded, getRealm(), count); + realmCache.getCache().addRevisioned(cached, realmCache.getStartupRevision()); + + return count; } @Override @@ -204,6 +223,10 @@ void registerOrganizationInvalidation(String orgId) { realmCache.registerInvalidation(orgId); } + private void registerCountInvalidation() { + realmCache.registerInvalidation(cacheKeyOrgCount(getRealm())); + } + private RealmModel getRealm() { RealmModel realm = session.getContext().getRealm(); if (realm == null) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java index 6768d72db9e8..65cc4b37c5fb 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java @@ -42,10 +42,13 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; import java.io.IOException; +import java.util.LinkedList; +import java.util.stream.IntStream; import org.junit.Test; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.common.Profile.Feature; +import org.keycloak.models.OrganizationModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.organization.OrganizationProvider; import org.keycloak.representations.idm.IdentityProviderRepresentation; @@ -452,13 +455,18 @@ public void testDeleteRealm() { @Test public void testCount() { - for (int i = 0; i < 10; i++) { - createOrganization("kc.org." + i); - } + List orgIds = IntStream.range(0, 10) + .mapToObj(i -> createOrganization("kc.org." + i).getId()) + .collect(Collectors.toList()); getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) session -> { OrganizationProvider orgProvider = session.getProvider(OrganizationProvider.class); assertEquals(10, orgProvider.count()); + + OrganizationModel org = orgProvider.getById(orgIds.get(0)); + orgProvider.remove(org); + + assertEquals(9, orgProvider.count()); }); } } From 3f92fc16040518caeec98ca21ce2a92cf8b0a024 Mon Sep 17 00:00:00 2001 From: vramik Date: Thu, 13 Jun 2024 09:20:11 +0200 Subject: [PATCH 4/4] getByMember Signed-off-by: vramik --- .../organization/InfinispanOrganizationProvider.java | 9 ++------- .../organization/jpa/JpaOrganizationProvider.java | 3 ++- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java index 4f08231b318a..0a17163143f3 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/organization/InfinispanOrganizationProvider.java @@ -154,8 +154,7 @@ public UserModel getMemberById(OrganizationModel organization, String id) { @Override public OrganizationModel getByMember(UserModel member) { - // Return cache delegate to ensure cache invalidation during write operations - return getCacheDelegate(orgDelegate.getByMember(member)); + return orgDelegate.getByMember(member); } @Override @@ -235,11 +234,7 @@ private RealmModel getRealm() { return realm; } - private OrganizationModel getCacheDelegate(OrganizationModel model) { - return model == null ? null : getById(model.getId()); - } - private Stream getCacheDelegates(Stream backendOrganizations) { - return backendOrganizations.map(this::getCacheDelegate); + return backendOrganizations.map(OrganizationModel::getId).map(this::getById); } } diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java index 1597a2ebe9d7..62ff4939265f 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java @@ -274,7 +274,8 @@ public OrganizationModel getByMember(UserModel member) { return null; } - return getById(orgId); + // need to go via the session to avoid bypassing the cache + return session.getProvider(OrganizationProvider.class).getById(orgId); } @Override