From 3985157f9f23344ce976cf9008503a0ee0b2b534 Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Tue, 14 May 2024 14:16:59 +0200 Subject: [PATCH] Make sure operations on a organization are based on realm they belong to Closes #28841 Signed-off-by: Martin Kanis --- .../jpa/JpaOrganizationProvider.java | 20 ++++++++++++++- .../OrganizationIdentityProviderTest.java | 25 +++++++++++++++++++ .../admin/OrganizationMemberTest.java | 23 +++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) 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 438846add9..6985bfbad0 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.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.models.utils.KeycloakModelUtils; import org.keycloak.organization.OrganizationProvider; import org.keycloak.utils.StringUtil; @@ -150,6 +149,11 @@ public class JpaOrganizationProvider implements OrganizationProvider { OrganizationEntity entity = getEntity(organization.getId()); OrganizationModel current = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName()); + // check the user and the organization belongs to the same realm + if (session.users().getUserById(session.realms().getRealm(entity.getRealmId()), user.getId()) == null) { + return false; + } + if (current == null) { session.setAttribute(OrganizationModel.class.getName(), organization); } @@ -283,6 +287,12 @@ public class JpaOrganizationProvider implements OrganizationProvider { throwExceptionIfObjectIsNull(identityProvider, "Identity provider"); OrganizationEntity organizationEntity = getEntity(organization.getId()); + + // check the identity provider and the organization belongs to the same realm + if (!checkOrgIdpAndRealm(organizationEntity, identityProvider)) { + return false; + } + String orgId = identityProvider.getOrganizationId(); if (organizationEntity.getId().equals(orgId)) { @@ -455,4 +465,12 @@ public class JpaOrganizationProvider implements OrganizationProvider { return null; } } + + // return true only if the organization realm and the identity provider realm is the same + private boolean checkOrgIdpAndRealm(OrganizationEntity orgEntity, IdentityProviderModel idp) { + RealmModel orgRealm = session.realms().getRealm(orgEntity.getRealmId()); + IdentityProviderModel orgIdpByAlias = orgRealm.getIdentityProviderByAlias(idp.getAlias()); + + return orgIdpByAlias != null && orgIdpByAlias.getInternalId().equals(idp.getInternalId()); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationIdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationIdentityProviderTest.java index 4dd6dfc93c..e034543c17 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationIdentityProviderTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationIdentityProviderTest.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.organization.admin; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertFalse; import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.NotFoundException; @@ -30,7 +31,10 @@ import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.admin.client.resource.OrganizationIdentityProviderResource; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.common.Profile.Feature; +import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.OrganizationModel; +import org.keycloak.models.RealmModel; +import org.keycloak.organization.OrganizationProvider; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; @@ -195,6 +199,27 @@ public class OrganizationIdentityProviderTest extends AbstractOrganizationTest { } } + @Test + public void testAddIdpFromDifferentRealm() { + String orgId = createOrganization().getId(); + IdentityProviderRepresentation idpRepresentation = createRep("master-identity-provider", "oidc"); + adminClient.realm("master").identityProviders().create(idpRepresentation).close(); + + getTestingClient().server(TEST_REALM_NAME).run(session -> { + OrganizationProvider provider = session.getProvider(OrganizationProvider.class); + OrganizationModel organization = provider.getById(orgId); + + RealmModel realm = session.realms().getRealmByName("master"); + IdentityProviderModel idp = realm.getIdentityProviderByAlias("master-identity-provider"); + + try { + assertFalse(provider.addIdentityProvider(organization, idp)); + } finally { + realm.removeIdentityProviderByAlias("master-identity-provider"); + } + }); + } + private IdentityProviderRepresentation createRep(String alias, String providerId) { IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationMemberTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationMemberTest.java index a874dae9c1..3b98298b2a 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationMemberTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationMemberTest.java @@ -45,7 +45,11 @@ import org.keycloak.admin.client.resource.OrganizationMemberResource; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.Profile.Feature; +import org.keycloak.models.OrganizationModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.organization.OrganizationProvider; import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -390,4 +394,23 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { assertThat(existing.get(1).getUsername(), is(equalTo("thejoker@neworg.org"))); } + @Test + public void testAddMemberFromDifferentRealm() { + String orgId = createOrganization().getId(); + + getTestingClient().server(TEST_REALM_NAME).run(session -> { + OrganizationProvider provider = session.getProvider(OrganizationProvider.class); + OrganizationModel organization = provider.getById(orgId); + + RealmModel realm = session.realms().getRealmByName("master"); + session.users().addUser(realm, "master-test-user"); + UserModel user = null; + try { + user = session.users().getUserByUsername(realm, "master-test-user"); + assertFalse(provider.addMember(organization, user)); + } finally { + session.users().removeUser(realm, user); + } + }); + } } \ No newline at end of file