From 50eaaf8e7a3bc5bf635967fbfb0aaa7159a6503e Mon Sep 17 00:00:00 2001 From: Aviad Lichtenstadt Date: Mon, 12 Feb 2024 15:47:19 +0200 Subject: [PATCH] Tenant level roles (#97) + tests + readme related to https://github.com/descope/etc/issues/2563 --- README.md | 2 + .../java/com/descope/model/roles/Role.java | 1 + .../com/descope/sdk/mgmt/RolesService.java | 11 +- .../sdk/mgmt/impl/RolesServiceImpl.java | 34 +++--- src/main/java/com/descope/utils/UriUtils.java | 11 +- .../sdk/mgmt/impl/RolesServiceImplTest.java | 101 ++++++++++++------ 6 files changed, 102 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index e6563c7e..d9bdbfe2 100644 --- a/README.md +++ b/README.md @@ -987,6 +987,8 @@ String name = "My Role"; String description = "Optional description to briefly explain what this role allows."; List permissionNames = Arrays.asList("My Updated Permission"); +// In case roles are on tenant scope, use the overloaded functions that has the tenantId parameter + try { rs.create(name, description, permissionNames); } catch (DescopeException de) { diff --git a/src/main/java/com/descope/model/roles/Role.java b/src/main/java/com/descope/model/roles/Role.java index fe765bb8..38ee1372 100644 --- a/src/main/java/com/descope/model/roles/Role.java +++ b/src/main/java/com/descope/model/roles/Role.java @@ -12,6 +12,7 @@ @AllArgsConstructor public class Role { private String name; + private String tenantId; private String description; private List permissionNames; private Long createdTime; diff --git a/src/main/java/com/descope/sdk/mgmt/RolesService.java b/src/main/java/com/descope/sdk/mgmt/RolesService.java index ad7d1522..e9803993 100644 --- a/src/main/java/com/descope/sdk/mgmt/RolesService.java +++ b/src/main/java/com/descope/sdk/mgmt/RolesService.java @@ -6,13 +6,18 @@ public interface RolesService { - void create(String name, String description, List permissionNames) - throws DescopeException; + void create(String name, String description, List permissionNames) throws DescopeException; + + void create(String name, String tenantId, String description, List permissionNames) throws DescopeException; - void update(String name, String newName, String description, List permissionNames) + void update(String name, String newName, String description, List permissionNames) throws DescopeException; + + void update(String name, String tenantId, String newName, String description, List permissionNames) throws DescopeException; void delete(String name) throws DescopeException; + void delete(String name, String tenantId) throws DescopeException; + RoleResponse loadAll() throws DescopeException; } diff --git a/src/main/java/com/descope/sdk/mgmt/impl/RolesServiceImpl.java b/src/main/java/com/descope/sdk/mgmt/impl/RolesServiceImpl.java index a650a6e2..e7570fde 100644 --- a/src/main/java/com/descope/sdk/mgmt/impl/RolesServiceImpl.java +++ b/src/main/java/com/descope/sdk/mgmt/impl/RolesServiceImpl.java @@ -23,12 +23,17 @@ class RolesServiceImpl extends ManagementsBase implements RolesService { } @Override - public void create(String name, String description, List permissionNames) + public void create(String name, String description, List permissionNames) throws DescopeException { + this.create(name, "", description, permissionNames); + } + + @Override + public void create(String name, String tenantId, String description, List permissionNames) throws DescopeException { if (StringUtils.isBlank(name)) { throw ServerCommonException.invalidArgument("Name"); } - Map request = mapOf("name", name, "description", description); + Map request = mapOf("name", name, "description", description, "tenantId", tenantId); if (permissionNames != null) { request.put("permissionNames", permissionNames); } @@ -39,32 +44,35 @@ public void create(String name, String description, List permissionNames @Override public void update(String name, String newName, String description, List permissionNames) throws DescopeException { + this.update(name, "", newName, description, permissionNames); + } + + @Override + public void update(String name, String tenantId, String newName, String description, List permissionNames) + throws DescopeException { if (StringUtils.isBlank(name)) { throw ServerCommonException.invalidArgument("Name"); } if (StringUtils.isBlank(newName)) { throw ServerCommonException.invalidArgument("NewName"); } - Map request = - mapOf( - "name", - name, - "newName", - newName, - "description", - description, - "permissionNames", - permissionNames); + Map request = mapOf("name", name, "newName", newName, "description", description, "permissionNames", + permissionNames, "tenantId", tenantId); ApiProxy apiProxy = getApiProxy(); apiProxy.post(getUri(MANAGEMENT_ROLES_UPDATE_LINK), request, Void.class); } @Override public void delete(String name) throws DescopeException { + this.delete(name, ""); + } + + @Override + public void delete(String name, String tenantId) throws DescopeException { if (StringUtils.isBlank(name)) { throw ServerCommonException.invalidArgument("Name"); } - Map request = mapOf("name", name); + Map request = mapOf("name", name, "tenantId", tenantId); ApiProxy apiProxy = getApiProxy(); apiProxy.post(getUri(MANAGEMENT_ROLES_DELETE_LINK), request, Void.class); } diff --git a/src/main/java/com/descope/utils/UriUtils.java b/src/main/java/com/descope/utils/UriUtils.java index b1f2ab19..83e5bd4a 100644 --- a/src/main/java/com/descope/utils/UriUtils.java +++ b/src/main/java/com/descope/utils/UriUtils.java @@ -46,11 +46,8 @@ public static Map> splitQuery(URL url) { return Collections.emptyMap(); } - return Arrays.stream(url.getQuery().split("&")) - .map(UriUtils::splitQueryParameter) - .collect(Collectors.groupingBy( - SimpleImmutableEntry::getKey, - LinkedHashMap::new, + return Arrays.stream(url.getQuery().split("&")).map(UriUtils::splitQueryParameter) + .collect(Collectors.groupingBy(SimpleImmutableEntry::getKey, LinkedHashMap::new, Collectors.mapping(Map.Entry::getValue, Collectors.toList()))); } @@ -59,8 +56,6 @@ public static SimpleImmutableEntry splitQueryParameter(String it final int idx = it.indexOf("="); final String key = idx > 0 ? it.substring(0, idx) : it; final String value = idx > 0 && it.length() > idx + 1 ? it.substring(idx + 1) : null; - return new SimpleImmutableEntry<>( - URLDecoder.decode(key, "UTF-8"), - URLDecoder.decode(value, "UTF-8")); + return new SimpleImmutableEntry<>(URLDecoder.decode(key, "UTF-8"), URLDecoder.decode(value, "UTF-8")); } } diff --git a/src/test/java/com/descope/sdk/mgmt/impl/RolesServiceImplTest.java b/src/test/java/com/descope/sdk/mgmt/impl/RolesServiceImplTest.java index fa647f68..828160ad 100644 --- a/src/test/java/com/descope/sdk/mgmt/impl/RolesServiceImplTest.java +++ b/src/test/java/com/descope/sdk/mgmt/impl/RolesServiceImplTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -23,6 +24,7 @@ import com.descope.sdk.TestUtils; import com.descope.sdk.mgmt.PermissionService; import com.descope.sdk.mgmt.RolesService; +import com.descope.sdk.mgmt.TenantService; import java.util.Arrays; import java.util.List; import org.assertj.core.api.Assertions; @@ -34,17 +36,12 @@ class RolesServiceImplTest { private final List mockPermissionNames = Arrays.asList("permission1", "permission2"); - private final List mockRole = - Arrays.asList( - Role.builder() - .name("someName") - .permissionNames(mockPermissionNames) - .description("someDesc") - .createdTime(1245667L) - .build()); + private final List mockRole = Arrays.asList(Role.builder().name("someName").permissionNames(mockPermissionNames) + .description("someDesc").createdTime(1245667L).build()); private final RoleResponse mockRoleResponse = new RoleResponse(mockRole); private RolesService rolesService; private PermissionService permissionService; + private TenantService tenantService; @BeforeEach void setUp() { @@ -52,14 +49,13 @@ void setUp() { ManagementServices mgmtServices = ManagementServiceBuilder.buildServices(client); this.rolesService = mgmtServices.getRolesService(); this.permissionService = mgmtServices.getPermissionService(); + this.tenantService = mgmtServices.getTenantService(); } @Test void testRolesForEmptyName() { - ServerCommonException thrown = - assertThrows( - ServerCommonException.class, - () -> rolesService.create("", "someDesc", mockPermissionNames)); + ServerCommonException thrown = assertThrows(ServerCommonException.class, + () -> rolesService.create("", "someDesc", mockPermissionNames)); assertNotNull(thrown); assertEquals("The Name argument is invalid", thrown.getMessage()); } @@ -69,8 +65,7 @@ void testRolesCreateSuccess() { ApiProxy apiProxy = mock(ApiProxy.class); doReturn(Void.class).when(apiProxy).post(any(), any(), any()); try (MockedStatic mockedApiProxyBuilder = mockStatic(ApiProxyBuilder.class)) { - mockedApiProxyBuilder.when( - () -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); + mockedApiProxyBuilder.when(() -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); rolesService.create("krishna", "", mockPermissionNames); verify(apiProxy, times(1)).post(any(), any(), any()); } @@ -78,10 +73,8 @@ void testRolesCreateSuccess() { @Test void testUpdateForEmptyName() { - ServerCommonException thrown = - assertThrows( - ServerCommonException.class, - () -> rolesService.update("", "", "", mockPermissionNames)); + ServerCommonException thrown = assertThrows(ServerCommonException.class, + () -> rolesService.update("", "", "", mockPermissionNames)); assertNotNull(thrown); assertEquals("The Name argument is invalid", thrown.getMessage()); } @@ -91,8 +84,7 @@ void testUpdateForSuccess() { ApiProxy apiProxy = mock(ApiProxy.class); doReturn(void.class).when(apiProxy).post(any(), any(), any()); try (MockedStatic mockedApiProxyBuilder = mockStatic(ApiProxyBuilder.class)) { - mockedApiProxyBuilder.when( - () -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); + mockedApiProxyBuilder.when(() -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); rolesService.update("Test", "name", "10", mockPermissionNames); verify(apiProxy, times(1)).post(any(), any(), any()); } @@ -100,18 +92,15 @@ void testUpdateForSuccess() { @Test void testUpdateForEmptyNewName() { - ServerCommonException thrown = - assertThrows( - ServerCommonException.class, - () -> rolesService.update("krishna", "", "", mockPermissionNames)); + ServerCommonException thrown = assertThrows(ServerCommonException.class, + () -> rolesService.update("krishna", "", "", mockPermissionNames)); assertNotNull(thrown); assertEquals("The NewName argument is invalid", thrown.getMessage()); } @Test void testDeleteForEmptyName() { - ServerCommonException thrown = - assertThrows(ServerCommonException.class, () -> rolesService.delete("")); + ServerCommonException thrown = assertThrows(ServerCommonException.class, () -> rolesService.delete("")); assertNotNull(thrown); assertEquals("The Name argument is invalid", thrown.getMessage()); } @@ -121,8 +110,7 @@ void testDeleteForSuccess() { ApiProxy apiProxy = mock(ApiProxy.class); doReturn(Void.class).when(apiProxy).post(any(), any(), any()); try (MockedStatic mockedApiProxyBuilder = mockStatic(ApiProxyBuilder.class)) { - mockedApiProxyBuilder.when( - () -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); + mockedApiProxyBuilder.when(() -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); rolesService.delete("someName"); verify(apiProxy, times(1)).post(any(), any(), any()); } @@ -133,17 +121,14 @@ void testLoadAllForSuccess() { ApiProxy apiProxy = mock(ApiProxy.class); doReturn(mockRoleResponse).when(apiProxy).get(any(), any()); try (MockedStatic mockedApiProxyBuilder = mockStatic(ApiProxyBuilder.class)) { - mockedApiProxyBuilder.when( - () -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); + mockedApiProxyBuilder.when(() -> ApiProxyBuilder.buildProxy(any(), any())).thenReturn(apiProxy); RoleResponse response = rolesService.loadAll(); Assertions.assertThat(response.getRoles().size()).isEqualTo(1); Assertions.assertThat(response.getRoles().get(0).getName()).isEqualTo("someName"); Assertions.assertThat(response.getRoles().get(0).getDescription()).isEqualTo("someDesc"); Assertions.assertThat(response.getRoles().get(0).getPermissionNames().size()).isEqualTo(2); - Assertions.assertThat(response.getRoles().get(0).getPermissionNames().get(0)) - .isEqualTo("permission1"); - Assertions.assertThat(response.getRoles().get(0).getPermissionNames().get(1)) - .isEqualTo("permission2"); + Assertions.assertThat(response.getRoles().get(0).getPermissionNames().get(0)).isEqualTo("permission1"); + Assertions.assertThat(response.getRoles().get(0).getPermissionNames().get(1)).isEqualTo("permission2"); } } @@ -182,4 +167,52 @@ void testFunctionalFullCycle() { permissionService.delete(p2); rolesService.delete(r1 + "1"); } + + @RetryingTest(value = 3, suspendForMs = 30000, onExceptions = RateLimitExceededException.class) + void testFunctionalFullCycleWithTenantId() { + String p1 = TestUtils.getRandomName("pt-").substring(0, 20); + String p2 = TestUtils.getRandomName("pt-").substring(0, 20); + String r1 = TestUtils.getRandomName("rt-").substring(0, 20); + permissionService.create(p1, "p1"); + permissionService.create(p2, "p2"); + String tid = tenantService.create(TestUtils.getRandomName("tnfr-").substring(0, 20), null); + rolesService.create(r1, tid, "ttt", Arrays.asList(p1, p2)); + RoleResponse roles = rolesService.loadAll(); + assertThat(roles.getRoles()).isNotEmpty(); + boolean found = false; + for (Role r : roles.getRoles()) { + if (r.getName().equals(r1)) { + found = true; + assertEquals("ttt", r.getDescription()); + assertEquals(tid, r.getTenantId()); + assertThat(r.getPermissionNames()).contains(p1, p2); + } + } + assertTrue(found); + rolesService.update(r1, tid, r1 + "1", "zzz", Arrays.asList(p1)); + roles = rolesService.loadAll(); + assertThat(roles.getRoles()).isNotEmpty(); + found = false; + for (Role r : roles.getRoles()) { + if (r.getName().equals(r1 + "1")) { + found = true; + assertEquals("zzz", r.getDescription()); + assertEquals(tid, r.getTenantId()); + assertThat(r.getPermissionNames()).containsExactly(p1); + } + } + assertTrue(found); + permissionService.delete(p1); + permissionService.delete(p2); + rolesService.delete(r1 + "1", tid); + found = false; + roles = rolesService.loadAll(); + for (Role r : roles.getRoles()) { + if (r.getName().equals(r1 + "1") && r.getTenantId().equals(tid)) { + found = true; + } + } + assertFalse(found); + tenantService.delete(tid); + } }