From 0007bad6f382e87fb214c640a710fc2226425d27 Mon Sep 17 00:00:00 2001 From: skyfalke Date: Mon, 27 May 2019 10:54:04 +0200 Subject: [PATCH] KEYCLOAK-10393 Fix permission ticket pagination in Authz Client KEYCLOAK-10393 Ensure idempotency of find method of permission ticket store --- .../client/resource/PermissionResource.java | 4 +-- .../jpa/store/JPAPermissionTicketStore.java | 2 +- .../authz/PermissionManagementTest.java | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/authz/client/src/main/java/org/keycloak/authorization/client/resource/PermissionResource.java b/authz/client/src/main/java/org/keycloak/authorization/client/resource/PermissionResource.java index b073fdb896..b4a5aa0f3f 100644 --- a/authz/client/src/main/java/org/keycloak/authorization/client/resource/PermissionResource.java +++ b/authz/client/src/main/java/org/keycloak/authorization/client/resource/PermissionResource.java @@ -213,8 +213,8 @@ public class PermissionResource { .param("requester", requester) .param("granted", granted == null ? null : granted.toString()) .param("returnNames", returnNames == null ? null : returnNames.toString()) - .param("firstResult", firstResult == null ? null : firstResult.toString()) - .param("maxResult", maxResult == null ? null : maxResult.toString()) + .param("first", firstResult == null ? null : firstResult.toString()) + .param("max", maxResult == null ? null : maxResult.toString()) .response().json(new TypeReference>(){}).execute(); } }; diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java index 1ee7a08eed..2c5fb12032 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPermissionTicketStore.java @@ -215,7 +215,7 @@ public class JPAPermissionTicketStore implements PermissionTicketStore { } }); - querybuilder.where(predicates.toArray(new Predicate[predicates.size()])).orderBy(builder.asc(root.get("resource").get("id"))); + querybuilder.where(predicates.toArray(new Predicate[predicates.size()])).orderBy(builder.asc(root.get("id"))); Query query = entityManager.createQuery(querybuilder); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java index 87172f67d7..356e72db04 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -379,4 +380,38 @@ public class PermissionManagementTest extends AbstractResourceServerTest { assertTrue(new String((HttpResponseException.class.cast(cause.getCause()).getBytes())).contains("invalid_scope")); } } + + @Test + public void testGetPermissionTicketWithPagination() throws Exception { + String[] scopes = {"ScopeA", "ScopeB", "ScopeC", "ScopeD"}; + ResourceRepresentation resource = addResource("Resource A", "kolo", true, scopes); + AuthzClient authzClient = getAuthzClient(); + PermissionResponse response = authzClient.protection("marta", "password").permission().create(new PermissionRequest(resource.getId(), scopes)); + AuthorizationRequest request = new AuthorizationRequest(); + request.setTicket(response.getTicket()); + request.setClaimToken(authzClient.obtainAccessToken("marta", "password").getToken()); + + try { + authzClient.authorization().authorize(request); + } catch (Exception e) { + + } + + // start with fetching the second half of all permission tickets + Collection expectedScopes = new ArrayList(Arrays.asList(scopes)); + List tickets = getAuthzClient().protection().permission().find(resource.getId(), null, null, null, null, true, 2, 2); + assertEquals("Returned number of permissions tickets must match the specified page size (i.e., 'maxResult').", 2, tickets.size()); + boolean foundScope = expectedScopes.remove(tickets.get(0).getScopeName()); + assertTrue("Returned set of permission tickets must be only a sub-set as per pagination offset and specified page size.", foundScope); + foundScope = expectedScopes.remove(tickets.get(1).getScopeName()); + assertTrue("Returned set of permission tickets must be only a sub-set as per pagination offset and specified page size.", foundScope); + + // fetch the first half of all permission tickets + tickets = getAuthzClient().protection().permission().find(resource.getId(), null, null, null, null, true, 0, 2); + assertEquals("Returned number of permissions tickets must match the specified page size (i.e., 'maxResult').", 2, tickets.size()); + foundScope = expectedScopes.remove(tickets.get(0).getScopeName()); + assertTrue("Returned set of permission tickets must be only a sub-set as per pagination offset and specified page size.", foundScope); + foundScope = expectedScopes.remove(tickets.get(1).getScopeName()); + assertTrue("Returned set of permission tickets must be only a sub-set as per pagination offset and specified page size.", foundScope); + } }