getAll() organization and organization members only returns the first 10 items

Closes #34975

Signed-off-by: Martin Kanis <mkanis@redhat.com>
This commit is contained in:
Martin Kanis 2024-11-21 14:51:49 +01:00 committed by Pedro Igor
parent a7e5a9f755
commit 05116f7951
13 changed files with 96 additions and 38 deletions

View File

@ -23,6 +23,11 @@ by the LDAP provider.
{project_name} now uses by default its database to discover other nodes of the same cluster, which removes the need of additional network related configurations. It is also a default that will work out-of-the-box in cloud environments.
= Deprecating `getAll()` methods in `Organizations` and `OrganizationMembers` APIs
`getAll()` methods in `Organizations` and `OrganizationMembers` APIs are now deprecated and will be removed in the next minor release.
Instead, use corresponding `list(first, max)` methods in `Organizations` and `OrganizationMembers` APIs.
For information on how to migrate, see the link:{upgradingguide_link}[{upgradingguide_name}], and the https://www.keycloak.org/server/caching[Configuring distributed caches] guide.

View File

@ -43,11 +43,27 @@ public interface OrganizationMembersResource {
* Return all members in the organization.
*
* @return a list containing the organization members.
* @Deprecated Use {@link org.keycloak.admin.client.resource.OrganizationMembersResource#list} instead.
*/
@Deprecated
@GET
@Produces(MediaType.APPLICATION_JSON)
List<MemberRepresentation> getAll();
/**
* Return members in the organization.
*
* @param first index of the first element (pagination offset).
* @param max the maximum number of results.
* @return a list containing organization members.
*/
@GET
@Produces(MediaType.APPLICATION_JSON)
List<MemberRepresentation> list(
@QueryParam("first") Integer firstResult,
@QueryParam("max") Integer maxResults
);
/**
* Return all organization members that match the specified filters.
*

View File

@ -48,11 +48,27 @@ public interface OrganizationsResource {
* Returns all organizations in the realm.
*
* @return a list containing the organizations.
* @Deprecated Use {@link org.keycloak.admin.client.resource.OrganizationsResource#list} instead.
*/
@Deprecated
@GET
@Produces(MediaType.APPLICATION_JSON)
List<OrganizationRepresentation> getAll();
/**
* Returns organizations in the realm.
*
* @param first index of the first element (pagination offset).
* @param max the maximum number of results.
* @return a list containing the organizations.
*/
@GET
@Produces(MediaType.APPLICATION_JSON)
List<OrganizationRepresentation> list(
@QueryParam("first") Integer firstResult,
@QueryParam("max") Integer maxResults
);
/**
* Returns all organizations that match the specified filter.
*

View File

@ -129,7 +129,7 @@ public class AdminEventTest extends AbstractEventTest {
orgRep.setAlias(orgRep.getName());
orgRep.addDomain(new OrganizationDomainRepresentation(orgRep.getName()));
testRealmResource().organizations().create(orgRep).close();
orgRep = testRealmResource().organizations().getAll().get(0);
orgRep = testRealmResource().organizations().list(-1, -1).get(0);
testRealmResource().organizations().get(orgRep.getId()).members().addMember(userId).close();
List<AdminEventRepresentation> events = events();
assertThat(events().size(), is(equalTo(4)));

View File

@ -134,7 +134,7 @@ public class OrganizationGroupTest extends AbstractOrganizationTest {
orgIds.add(createOrganization("org-" + i).getId());
}
assertEquals(orgIds.size(), testRealm().organizations().getAll().size());
assertEquals(orgIds.size(), testRealm().organizations().list(-1, -1).size());
assertTrue(testRealm().groups().groups().stream().map(GroupRepresentation::getId).noneMatch(orgIds::contains));
}

View File

@ -292,7 +292,7 @@ public class OrganizationInvitationLinkTest extends AbstractOrganizationTest {
private void registerUser(OrganizationResource organization, String expectedEmail, String email) throws MessagingException, IOException {
String link = getInvitationLinkFromEmail();
driver.navigate().to(link);
Assert.assertFalse(organization.members().getAll().stream().anyMatch(actual -> email.equals(actual.getEmail())));
Assert.assertFalse(organization.members().list(-1, -1).stream().anyMatch(actual -> email.equals(actual.getEmail())));
registerPage.assertCurrent(organizationName);
assertThat(registerPage.getEmail(), equalTo(expectedEmail));
registerPage.register("firstName", "lastName", email,
@ -307,7 +307,7 @@ public class OrganizationInvitationLinkTest extends AbstractOrganizationTest {
String link = getInvitationLinkFromEmail(user.getFirstName(), user.getLastName());
driver.navigate().to(link);
// not yet a member
Assert.assertFalse(organization.members().getAll().stream().anyMatch(actual -> user.getId().equals(actual.getId())));
Assert.assertFalse(organization.members().list(-1, -1).stream().anyMatch(actual -> user.getId().equals(actual.getId())));
// confirm the intent of membership
assertThat(driver.getPageSource(), containsString("You are about to join organization " + organizationName));
assertThat(infoPage.getInfo(), containsString("By clicking on the link below, you will become a member of the " + organizationName + " organization:"));

View File

@ -21,7 +21,6 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
@ -34,6 +33,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
@ -47,6 +47,7 @@ import jakarta.ws.rs.core.Response.Status;
import java.io.IOException;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.junit.Assert;
import org.junit.Test;
@ -107,17 +108,25 @@ public class OrganizationTest extends AbstractOrganizationTest {
public void testGetAll() {
List<OrganizationRepresentation> expected = new ArrayList<>();
for (int i = 0; i < 5; i++) {
for (int i = 0; i < 15; i++) {
OrganizationRepresentation organization = createOrganization("kc.org." + i);
expected.add(organization);
organization.setAttributes(Map.of("foo", List.of("foo")));
testRealm().organizations().get(organization.getId()).update(organization).close();
}
List<OrganizationRepresentation> existing = testRealm().organizations().getAll();
List<OrganizationRepresentation> existing = testRealm().organizations().list(-1, -1);
assertFalse(existing.isEmpty());
assertThat(expected, containsInAnyOrder(existing.toArray()));
assertThat(existing, containsInAnyOrder(expected.toArray()));
Assert.assertTrue(existing.stream().map(OrganizationRepresentation::getAttributes).filter(Objects::nonNull).findAny().isEmpty());
List<OrganizationRepresentation> concatenatedList = Stream.of(
testRealm().organizations().list(0, 5),
testRealm().organizations().list(5, 5),
testRealm().organizations().list(10, 5))
.flatMap(Collection::stream).toList();
assertThat(concatenatedList, containsInAnyOrder(expected.toArray()));
}
@Test
@ -424,7 +433,7 @@ public class OrganizationTest extends AbstractOrganizationTest {
assertEquals(Status.NOT_FOUND.getStatusCode(), response.getStatus());
}
try {
testRealm().organizations().getAll();
testRealm().organizations().list(-1, -1);
fail("Expected NotFoundException");
} catch (NotFoundException expected) {
}
@ -457,7 +466,7 @@ public class OrganizationTest extends AbstractOrganizationTest {
createOrganization(realmRes, "test-org", "test.org");
List<OrganizationRepresentation> orgs = realmRes.organizations().getAll();
List<OrganizationRepresentation> orgs = realmRes.organizations().list(-1, -1);
assertThat(orgs, hasSize(1));
IdentityProviderRepresentation broker = bc.setUpIdentityProvider();

View File

@ -160,14 +160,14 @@ public class OrganizationAdminPermissionsTest extends AbstractOrganizationTest {
//get members
try {
//we should get 403, not 400 or 404 etc.
realmUserResource.organizations().get("non-existing").members().getAll();
realmUserResource.organizations().get("non-existing").members().list(-1, -1);
fail("Expected ForbiddenException");
} catch (ForbiddenException expected) {}
try {
realmUserResource.organizations().get(orgId).members().getAll();
realmUserResource.organizations().get(orgId).members().list(-1, -1);
fail("Expected ForbiddenException");
} catch (ForbiddenException expected) {}
assertThat(realmAdminResource.organizations().get(orgId).members().getAll(), Matchers.notNullValue());
assertThat(realmAdminResource.organizations().get(orgId).members().list(-1, -1), Matchers.notNullValue());
//get member
try {

View File

@ -619,7 +619,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz
loginOrgIdp("external", bc.getUserEmail(), true, true);
assertThat(organization.members().getAll(), Matchers.empty());
assertThat(organization.members().list(-1, -1), Matchers.empty());
UserRepresentation user = testRealm().users().searchByEmail(bc.getUserEmail(), true).get(0);
testRealm().users().get(user.getId()).remove();

View File

@ -300,8 +300,8 @@ public class OrganizationCacheTest extends AbstractOrganizationTest {
getCleanup().addCleanup(testRealm().identityProviders().get("alias")::remove);
}
String orgaId = testRealm().organizations().getAll().get(0).getId();
String orgbId = testRealm().organizations().getAll().get(1).getId();
String orgaId = testRealm().organizations().list(-1, -1).get(0).getId();
String orgbId = testRealm().organizations().list(-1, -1).get(1).getId();
for (int i = 0; i < 5; i++) {
final String aliasA = "org-idp-" + i;
@ -376,7 +376,7 @@ public class OrganizationCacheTest extends AbstractOrganizationTest {
getCleanup().addCleanup(testRealm().identityProviders().get("alias")::remove);
}
String orgaId = testRealm().organizations().getAll().get(0).getId();
String orgaId = testRealm().organizations().list(-1, -1).get(0).getId();
for (int i = 10; i < 20; i++) {
testRealm().organizations().get(orgaId).identityProviders().addIdentityProvider("idp-alias-" + i);
}

View File

@ -117,7 +117,7 @@ public class OrganizationExportTest extends AbstractOrganizationTest {
assertTrue(importedRealm.isOrganizationsEnabled());
List<OrganizationRepresentation> organizations = testRealm().organizations().getAll();
List<OrganizationRepresentation> organizations = testRealm().organizations().list(-1, -1);
assertEquals(expectedOrganizations.size(), organizations.size());
// id, name, alias, description and redirectUrl should have all been preserved.
assertThat(organizations.stream().map(OrganizationRepresentation::getId).toList(),
@ -145,7 +145,7 @@ public class OrganizationExportTest extends AbstractOrganizationTest {
for (OrganizationRepresentation orgRep : organizations) {
OrganizationResource organization = testRealm().organizations().get(orgRep.getId());
List<String> members = organization.members().getAll().stream().map(UserRepresentation::getEmail).toList();
List<String> members = organization.members().list(-1, -1).stream().map(UserRepresentation::getEmail).toList();
assertEquals(members.size(), expectedUnmanagedMembers.get(orgRep.getName()).size() + expectedManagedMembers.get(orgRep.getName()).size());
assertTrue(members.containsAll(expectedUnmanagedMembers.get(orgRep.getName())));
assertTrue(members.containsAll(expectedManagedMembers.get(orgRep.getName())));
@ -172,14 +172,14 @@ public class OrganizationExportTest extends AbstractOrganizationTest {
try (Response response = testRealm().organizations().create(orgRep)) {
assertEquals(Response.Status.CREATED.getStatusCode(), response.getStatus());
}
List<OrganizationRepresentation> orgs = testRealm().organizations().getAll();
List<OrganizationRepresentation> orgs = testRealm().organizations().list(-1, -1);
assertEquals(1, orgs.size());
RealmRepresentation importedRealm = exportRemoveImportRealm();
assertTrue(importedRealm.isOrganizationsEnabled());
orgs = testRealm().organizations().getAll();
orgs = testRealm().organizations().list(-1, -1);
assertEquals(1, orgs.size());
assertEquals("acme", orgs.get(0).getName());
}

View File

@ -89,8 +89,8 @@ public class OrganizationOIDCProtocolMapperTest extends AbstractOrganizationTest
orgb.members().addMember(member.getId()).close();
Assert.assertTrue(orga.members().getAll().stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orgb.members().getAll().stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orga.members().list(-1, -1).stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orgb.members().list(-1, -1).stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
oauth.clientId("direct-grant");
oauth.scope("openid organization:*");

View File

@ -18,6 +18,7 @@
package org.keycloak.testsuite.organization.member;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
@ -33,6 +34,7 @@ import static org.junit.Assert.fail;
import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import jakarta.ws.rs.BadRequestException;
@ -40,6 +42,7 @@ import jakarta.ws.rs.NotFoundException;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import java.io.IOException;
import java.util.stream.Stream;
import org.hamcrest.Matchers;
import org.junit.Test;
@ -51,6 +54,7 @@ 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.AbstractUserRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.representations.idm.MemberRepresentation;
import org.keycloak.representations.idm.MembershipType;
@ -134,11 +138,11 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());
List<UserRepresentation> expected = new ArrayList<>();
for (int i = 0; i < 5; i++) {
for (int i = 0; i < 15; i++) {
expected.add(addMember(organization, "member-" + i + "@neworg.org"));
}
List<MemberRepresentation> existing = organization.members().getAll();
List<MemberRepresentation> existing = organization.members().list(-1, -1);
assertFalse(existing.isEmpty());
assertEquals(expected.size(), existing.size());
for (UserRepresentation expectedRep : expected) {
@ -151,6 +155,14 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
assertEquals(expectedRep.getLastName(), existingRep.getLastName());
assertTrue(expectedRep.isEnabled());
}
List<String> concatenatedList = Stream.of(
organization.members().list(0, 5).stream().map(AbstractUserRepresentation::getId).toList(),
organization.members().list(5, 5).stream().map(AbstractUserRepresentation::getId).toList(),
organization.members().list(10, 5).stream().map(AbstractUserRepresentation::getId).toList())
.flatMap(Collection::stream).toList();
assertThat(concatenatedList, containsInAnyOrder(expected.stream().map(AbstractUserRepresentation::getId).toArray()));
}
@Test
@ -177,7 +189,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
assertThat(existingOrg.isEnabled(), is(false));
// now fetch all users from the org - unmanaged users should still be enabled, but managed ones should not.
List<MemberRepresentation> existing = organization.members().getAll();
List<MemberRepresentation> existing = organization.members().list(-1, -1);
assertThat(existing, not(empty()));
assertThat(existing, hasSize(6));
for (UserRepresentation user : existing) {
@ -467,7 +479,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
}
//check the federated user is not a member
assertThat(testRealm().organizations().get(id).members().getAll(), hasSize(0));
assertThat(testRealm().organizations().get(id).members().list(-1, -1), hasSize(0));
}
@Test
@ -481,8 +493,8 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
orgb.members().addMember(member.getId()).close();
Assert.assertTrue(orga.members().getAll().stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orgb.members().getAll().stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orga.members().list(-1, -1).stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
Assert.assertTrue(orgb.members().list(-1, -1).stream().map(UserRepresentation::getId).anyMatch(member.getId()::equals));
String orgbId = orgb.toRepresentation().getId();
String orgaId = orga.toRepresentation().getId();
List<String> memberOfOrgs = orga.members().member(member.getId()).getOrganizations().stream().map(OrganizationRepresentation::getId).toList();
@ -494,7 +506,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
public void testManagedMemberOnlyRemovedFromHomeOrganization() {
OrganizationResource orga = testRealm().organizations().get(createOrganization("org-a").getId());
assertBrokerRegistration(orga, bc.getUserEmail(), "managed-org-a@org-a.org");
UserRepresentation memberOrgA = orga.members().getAll().get(0);
UserRepresentation memberOrgA = orga.members().list(-1, -1).get(0);
realmsResouce().realm(bc.consumerRealmName()).users().get(memberOrgA.getId()).logout();
realmsResouce().realm(bc.providerRealmName()).logoutAll();
@ -506,21 +518,21 @@ public class OrganizationMemberTest extends AbstractOrganizationTest {
.build();
realmsResouce().realm(bc.providerRealmName()).users().create(memberOrgB).close();
assertBrokerRegistration(orgb, memberOrgB.getUsername(), "managed-org-b@org-b.org");
memberOrgB = orgb.members().getAll().get(0);
memberOrgB = orgb.members().list(-1, -1).get(0);
orga.members().addMember(memberOrgB.getId()).close();
assertThat(orga.members().getAll().size(), is(2));
assertThat(orga.members().list(-1, -1).size(), is(2));
OrganizationMemberResource memberOrgBInOrgA = orga.members().member(memberOrgB.getId());
memberOrgB = memberOrgBInOrgA.toRepresentation();
memberOrgBInOrgA.delete().close();
assertThat(orga.members().getAll().size(), is(1));
assertThat(orga.members().getAll().get(0).getId(), is(memberOrgA.getId()));
assertThat(orgb.members().getAll().size(), is(1));
assertThat(orga.members().list(-1, -1).size(), is(1));
assertThat(orga.members().list(-1, -1).get(0).getId(), is(memberOrgA.getId()));
assertThat(orgb.members().list(-1, -1).size(), is(1));
orgb.members().member(memberOrgB.getId()).delete().close();
assertThat(orga.members().getAll().size(), is(1));
assertThat(orga.members().getAll().get(0).getId(), is(memberOrgA.getId()));
assertThat(orgb.members().getAll().size(), is(0));
assertThat(orga.members().list(-1, -1).size(), is(1));
assertThat(orga.members().list(-1, -1).get(0).getId(), is(memberOrgA.getId()));
assertThat(orgb.members().list(-1, -1).size(), is(0));
}
@Test