From 694ffaf289315aa548fc1354d5f30861b006ad90 Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Mon, 27 May 2024 17:01:09 -0300 Subject: [PATCH] Allow organizations in different realms to have the same domain Closes #29886 Signed-off-by: Stefan Guilhen --- .../entities/OrganizationDomainEntity.java | 21 ++++++++++---- .../jpa/entities/OrganizationEntity.java | 2 ++ .../jpa/JpaOrganizationProvider.java | 8 +++--- .../organization/jpa/OrganizationAdapter.java | 7 +++-- .../META-INF/jpa-changelog-25.0.0.xml | 3 ++ .../models/OrganizationDomainModel.java | 2 +- .../organization/utils/Organizations.java | 2 +- .../admin/AbstractOrganizationTest.java | 28 ++++++++++++++----- .../organization/admin/OrganizationTest.java | 5 +++- 9 files changed, 55 insertions(+), 23 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java index 8af63978e3..cbc95c776b 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java @@ -17,14 +17,14 @@ package org.keycloak.models.jpa.entities; +import jakarta.persistence.Access; +import jakarta.persistence.AccessType; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.FetchType; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; -import jakarta.persistence.NamedQueries; -import jakarta.persistence.NamedQuery; import jakarta.persistence.Table; /** @@ -34,12 +34,13 @@ import jakarta.persistence.Table; */ @Entity @Table(name="ORG_DOMAIN") -@NamedQueries({ - @NamedQuery(name="getByName", query="select o from OrganizationDomainEntity o where o.name = :name") -}) public class OrganizationDomainEntity { @Id + @Column(name = "ID", length = 36) + @Access(AccessType.PROPERTY) + private String id; + @Column(name="NAME") protected String name; @@ -50,6 +51,14 @@ public class OrganizationDomainEntity { @JoinColumn(name = "ORG_ID") private OrganizationEntity organization; + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + public String getName() { return this.name; } @@ -81,7 +90,7 @@ public class OrganizationDomainEntity { if (!(o instanceof OrganizationDomainEntity)) return false; OrganizationDomainEntity that = (OrganizationDomainEntity) o; - return name != null && name.equals(that.getName()); + return id != null && id.equals(that.getId()); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java index 57f3731095..3ce9cd0288 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java @@ -37,6 +37,8 @@ import jakarta.persistence.Table; @NamedQueries({ @NamedQuery(name="getByRealm", query="select o from OrganizationEntity o where o.realmId = :realmId order by o.name ASC"), @NamedQuery(name="getByOrgName", query="select distinct o from OrganizationEntity o where o.realmId = :realmId AND o.name = :name"), + @NamedQuery(name="getByDomainName", query="select distinct o from OrganizationEntity o inner join OrganizationDomainEntity d ON o.id = d.organization.id" + + " where o.realmId = :realmId AND d.name = :name"), @NamedQuery(name="getByNameOrDomain", query="select distinct o from OrganizationEntity o inner join OrganizationDomainEntity d ON o.id = d.organization.id" + " where o.realmId = :realmId AND (o.name = :search OR d.name = :search) order by o.name ASC"), @NamedQuery(name="getByNameOrDomainContained", query="select distinct o from OrganizationEntity o inner join OrganizationDomainEntity d ON o.id = d.organization.id" + 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 ddf1b59663..e8a0b39be5 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 @@ -54,7 +54,6 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserProvider; import org.keycloak.models.jpa.entities.GroupAttributeEntity; import org.keycloak.models.jpa.entities.GroupEntity; -import org.keycloak.models.jpa.entities.OrganizationDomainEntity; import org.keycloak.models.jpa.entities.OrganizationEntity; import org.keycloak.organization.OrganizationProvider; import org.keycloak.utils.StringUtil; @@ -190,11 +189,12 @@ public class JpaOrganizationProvider implements OrganizationProvider { @Override public OrganizationModel getByDomainName(String domain) { - TypedQuery query = em.createNamedQuery("getByName", OrganizationDomainEntity.class); + TypedQuery query = em.createNamedQuery("getByDomainName", OrganizationEntity.class); + query.setParameter("realmId", this.realm.getId()); query.setParameter("name", domain.toLowerCase()); try { - OrganizationDomainEntity entity = query.getSingleResult(); - return new OrganizationAdapter(realm, entity.getOrganization(), this); + OrganizationEntity entity = query.getSingleResult(); + return new OrganizationAdapter(realm, entity, this); } catch (NoResultException nre) { return null; } diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java index 5eb0d5a099..5da7af8077 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java @@ -143,7 +143,7 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel(this.entity.getDomains())) { // update the existing domain (for now, only the verified flag can be changed). if (modelMap.containsKey(domainEntity.getName())) { - domainEntity.setVerified(modelMap.get(domainEntity.getName()).getVerified()); + domainEntity.setVerified(modelMap.get(domainEntity.getName()).isVerified()); modelMap.remove(domainEntity.getName()); } else { // remove domain that is not found in the new set. @@ -161,8 +161,9 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel + + + diff --git a/server-spi/src/main/java/org/keycloak/models/OrganizationDomainModel.java b/server-spi/src/main/java/org/keycloak/models/OrganizationDomainModel.java index 4cde7e5f45..45db1facba 100644 --- a/server-spi/src/main/java/org/keycloak/models/OrganizationDomainModel.java +++ b/server-spi/src/main/java/org/keycloak/models/OrganizationDomainModel.java @@ -42,7 +42,7 @@ public class OrganizationDomainModel implements Serializable { return this.name; } - public boolean getVerified() { + public boolean isVerified() { return this.verified; } diff --git a/services/src/main/java/org/keycloak/organization/utils/Organizations.java b/services/src/main/java/org/keycloak/organization/utils/Organizations.java index 1c58c8c923..512af7d5c3 100644 --- a/services/src/main/java/org/keycloak/organization/utils/Organizations.java +++ b/services/src/main/java/org/keycloak/organization/utils/Organizations.java @@ -155,7 +155,7 @@ public class Organizations { public static OrganizationDomainRepresentation toRepresentation(OrganizationDomainModel model) { OrganizationDomainRepresentation representation = new OrganizationDomainRepresentation(); representation.setName(model.getName()); - representation.setVerified(model.getVerified()); + representation.setVerified(model.isVerified()); return representation; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java index 1551bc3a74..463f815170 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java @@ -98,19 +98,25 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest { return createOrganization(name, name + ".org"); } - protected OrganizationRepresentation createOrganization(String name, String... orgDomain) { - return createOrganization(testRealm(), getCleanup(), name, brokerConfigFunction.apply(name).setUpIdentityProvider(), orgDomain); + protected OrganizationRepresentation createOrganization(String name, String... orgDomains) { + return createOrganization(testRealm(), name, orgDomains); } - protected static OrganizationRepresentation createOrganization(RealmResource testRealm, TestCleanup testCleanup, String name, IdentityProviderRepresentation broker, String... orgDomain) { - OrganizationRepresentation org = createRepresentation(name, orgDomain); + protected OrganizationRepresentation createOrganization(RealmResource realm, String name, String... orgDomains) { + return createOrganization(realm, getCleanup(), name, brokerConfigFunction.apply(name).setUpIdentityProvider(), orgDomains); + } + + protected static OrganizationRepresentation createOrganization(RealmResource testRealm, TestCleanup testCleanup, String name, + IdentityProviderRepresentation broker, String... orgDomains) { + OrganizationRepresentation org = createRepresentation(name, orgDomains); String id; try (Response response = testRealm.organizations().create(org)) { assertEquals(Status.CREATED.getStatusCode(), response.getStatus()); id = ApiUtil.getCreatedId(response); } - broker.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, org.getDomains().iterator().next().getName()); + // set the idp domain to the first domain used to create the org. + broker.getConfig().put(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, orgDomains[0]); testRealm.identityProviders().create(broker).close(); testCleanup.addCleanup(testRealm.identityProviders().get(broker.getAlias())::remove); testRealm.organizations().get(id).identityProviders().addIdentityProvider(broker.getAlias()).close(); @@ -226,6 +232,14 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest { } protected BrokerConfiguration createBrokerConfiguration() { - return new KcOidcBrokerConfiguration(); - }; + return new KcOidcBrokerConfiguration() { + @Override + public RealmRepresentation createProviderRealm() { + // enable organizations in the provider realm too just for testing purposes. + RealmRepresentation realmRep = super.createProviderRealm(); + realmRep.setOrganizationsEnabled(true); + return realmRep; + } + }; + } } 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 2bef33356a..e4fc98b317 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 @@ -355,7 +355,7 @@ public class OrganizationTest extends AbstractOrganizationTest { } expectedNewOrgBrDomain.setName("acme.com"); - // create another org and attempt to set the same internet domain during update - should not be possible. + // create another org in the same realm and attempt to set the same internet domain during update - should not be possible. OrganizationRepresentation anotherOrg = createOrganization("another-org"); anotherOrg.addDomain(expectedNewOrgDomain); organization = testRealm().organizations().get(anotherOrg.getId()); @@ -363,6 +363,9 @@ public class OrganizationTest extends AbstractOrganizationTest { assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); } + // create another org in a different realm with the same internet domain - should be allowed. + createOrganization(adminClient.realm(bc.providerRealmName()), "testorg", "acme.com"); + // try to remove a domain organization = testRealm().organizations().get(existing.getId()); existing.removeDomain(existingNewOrgDomain);