getAll() organization and organization members only returns the first…

… 10 items Closes keycloak#34975 Signed-off-by: Martin Kanis <[email protected]> (cherry picked from commit 7e3e46d)
miquelsi · Nov 25, 2024 · ea13176 · ea13176
1 parent 69001b3
commit ea13176
Show file tree

Hide file tree

Showing 13 changed files with 98 additions and 36 deletions.
diff --git a/docs/documentation/upgrading/topics/changes/changes-26_0_7.adoc b/docs/documentation/upgrading/topics/changes/changes-26_0_7.adoc
@@ -0,0 +1,4 @@
+= 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 major release.
+Instead, use corresponding `list(first, max)` methods in `Organizations` and `OrganizationMembers` APIs.
diff --git a/docs/documentation/upgrading/topics/changes/changes.adoc b/docs/documentation/upgrading/topics/changes/changes.adoc
@@ -1,6 +1,10 @@
 [[migration-changes]]
 == Migration Changes
 
+=== Migrating to 26.0.7
+
+include::changes-26_0_7.adoc[leveloffset=3]
+
 === Migrating to 26.0.0
 
 include::changes-26_0_0.adoc[leveloffset=3]

diff --git a/...-client/src/main/java/org/keycloak/admin/client/resource/OrganizationMembersResource.java b/...-client/src/main/java/org/keycloak/admin/client/resource/OrganizationMembersResource.java
@@ -42,11 +42,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.
      *

diff --git a/.../admin-client/src/main/java/org/keycloak/admin/client/resource/OrganizationsResource.java b/.../admin-client/src/main/java/org/keycloak/admin/client/resource/OrganizationsResource.java
@@ -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.
      *

diff --git a/...s/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java b/...s/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java
@@ -134,7 +134,7 @@ public void testOrganizationGroupsNotAvailableFromGroupAPI() {
             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));
     }
 

diff --git a/...c/test/java/org/keycloak/testsuite/organization/admin/OrganizationInvitationLinkTest.java b/...c/test/java/org/keycloak/testsuite/organization/admin/OrganizationInvitationLinkTest.java
@@ -292,7 +292,7 @@ private void registerUser(OrganizationResource organization, String email) throw
     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 @@ private void acceptInvitation(OrganizationResource organization, UserRepresentat
         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:"));

diff --git a/.../tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java b/.../tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java
@@ -34,6 +34,7 @@
 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 +48,7 @@
 
 import java.io.IOException;
 import java.util.stream.IntStream;
+import java.util.stream.Stream;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -107,17 +109,25 @@ public void testGet() {
     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 +434,7 @@ public void testDisabledOrganizationProvider() throws IOException {
                 assertEquals(Status.NOT_FOUND.getStatusCode(), response.getStatus());
             }
             try {
-                testRealm().organizations().getAll();
+                testRealm().organizations().list(-1, -1);
                 fail("Expected NotFoundException");
             } catch (NotFoundException expected) {
             }
@@ -457,7 +467,7 @@ public void testDeleteRealm() {
 
             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();

diff --git a/...test/java/org/keycloak/testsuite/organization/authz/OrganizationAdminPermissionsTest.java b/...test/java/org/keycloak/testsuite/organization/authz/OrganizationAdminPermissionsTest.java
@@ -160,14 +160,14 @@ public void testManageRealmRole() throws Exception {
             //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 {

diff --git a/...t/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java b/...t/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java
@@ -619,7 +619,7 @@ public void testRealmLevelBrokerNotImpactedByOrganizationFlow() {
 
         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();

diff --git a/...s/base/src/test/java/org/keycloak/testsuite/organization/cache/OrganizationCacheTest.java b/...s/base/src/test/java/org/keycloak/testsuite/organization/cache/OrganizationCacheTest.java
@@ -300,8 +300,8 @@ public void testCacheIDPByOrg() {
             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 void testCacheIDPForLogin() {
             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);
         }

diff --git a/...rc/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java b/...rc/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java
@@ -117,7 +117,7 @@ public void testExport() {
 
         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 void testExport() {
 
         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 void testExportImportEmptyOrg() {
         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());
     }

diff --git a/...t/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java b/...t/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java
@@ -89,8 +89,8 @@ public void testPasswordGrantType() throws Exception {
 
         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:*");

diff --git a/...base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java b/...base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java
@@ -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,13 +34,15 @@
 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;
 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.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.OrganizationRepresentation;
@@ -133,11 +137,11 @@ public void testGetAll() {
         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) {
@@ -150,6 +154,14 @@ public void testGetAll() {
             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
@@ -176,7 +188,7 @@ public void testGetAllDisabledOrganization() {
         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) {
@@ -461,7 +473,7 @@ public void testUserFederatedBeforeTheIDPBoundWithAnOrgIsNotMember() {
         }
 
         //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
@@ -475,8 +487,8 @@ public void testMemberInMultipleOrganizations() {
 
         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();
@@ -488,7 +500,7 @@ public void testMemberInMultipleOrganizations() {
     public void testManagedMemberOnlyRemovedFromHomeOrganization() {
         OrganizationResource orga = testRealm().organizations().get(createOrganization("org-a").getId());
         assertBrokerRegistration(orga, bc.getUserEmail(), "[email protected]");
-        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();
 
@@ -500,21 +512,21 @@ public void testManagedMemberOnlyRemovedFromHomeOrganization() {
                 .build();
         realmsResouce().realm(bc.providerRealmName()).users().create(memberOrgB).close();
         assertBrokerRegistration(orgb, memberOrgB.getUsername(), "[email protected]");
-        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