Skip to content

Commit

Permalink
Merge pull request #522 from folio-org/MODFQMMGR-563
Browse files Browse the repository at this point in the history
MODFQMMGR-563: Add /query/contents/privileged endpoint for list export
  • Loading branch information
bvsharp authored Nov 4, 2024
2 parents 86287c9 + c5a5964 commit 1a6b13b
Show file tree
Hide file tree
Showing 12 changed files with 182 additions and 45 deletions.
12 changes: 12 additions & 0 deletions descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@
"permissionsRequired": ["fqm.query.async.results.post"],
"modulePermissions": ["perms.users.get", "user-tenants.collection.get", "consortia.user-tenants.collection.get", "permissions.users.item.get", "configuration.entries.collection.get"]
},
{
"methods": ["POST"],
"pathPattern": "/query/contents/privileged",
"permissionsRequired": ["fqm.query.privileged.async.results.post"],
"modulePermissions": ["perms.users.get", "user-tenants.collection.get", "consortia.user-tenants.collection.get", "permissions.users.item.get", "configuration.entries.collection.get"]
},
{
"methods": ["POST"],
"pathPattern": "/query",
Expand Down Expand Up @@ -196,6 +202,12 @@
"description": "Retrieve results of a submitted query",
"visible": true
},
{
"permissionName": "fqm.query.privileged.async.results.post",
"displayName": "FQM - Retrieve contents (privileged)",
"description": "FQM - Retrieve contents (privileged)",
"visible": false
},
{
"permissionName": "fqm.query.async.delete",
"displayName": "FQM - Delete a submitted query",
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/folio/fqm/resource/FqlQueryController.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,17 @@ public ResponseEntity<QueryDetails> getQuery(UUID queryId, Boolean includeResult
@Override
public ResponseEntity<List<Map<String, Object>>> getContents(ContentsRequest contentsRequest) {
return ResponseEntity.ok(queryManagementService.getContents(contentsRequest.getEntityTypeId(),
contentsRequest.getFields(), contentsRequest.getIds(), Boolean.TRUE.equals(contentsRequest.getLocalize())));
contentsRequest.getFields(), contentsRequest.getIds(), null, Boolean.TRUE.equals(contentsRequest.getLocalize()), false));
}

@EntityTypePermissionsRequired(ContentsRequest.class)
@Override
public ResponseEntity<List<Map<String, Object>>> getContentsPrivileged(ContentsRequest contentsRequest) {
if (contentsRequest.getUserId() == null) {
return ResponseEntity.badRequest().build();
}
return ResponseEntity.ok(queryManagementService.getContents(contentsRequest.getEntityTypeId(),
contentsRequest.getFields(), contentsRequest.getIds(), contentsRequest.getUserId(), Boolean.TRUE.equals(contentsRequest.getLocalize()), true));
}

/**
Expand Down
29 changes: 21 additions & 8 deletions src/main/java/org/folio/fqm/service/CrossTenantQueryService.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;

@Service
@RequiredArgsConstructor
Expand All @@ -36,11 +37,22 @@ public class CrossTenantQueryService {
* @return List of tenants to query
*/
public List<String> getTenantsToQuery(EntityType entityType) {
return getTenantsToQuery(entityType, executionContext.getUserId());
}

/**
* Retrieve list of tenants to run query against for a specified user.
*
* @param entityType Entity type definition
* @param userId ID of user to retrieve tenant affiliations for
* @return List of tenants to query
*/
public List<String> getTenantsToQuery(EntityType entityType, UUID userId) {
if (!Boolean.TRUE.equals(entityType.getCrossTenantQueriesEnabled())
&& !COMPOSITE_INSTANCES_ID.equals(entityType.getId())) {
return List.of(executionContext.getTenantId());
}
return getTenants(entityType);
return getTenants(entityType, userId);
}

/**
Expand All @@ -51,10 +63,11 @@ public List<String> getTenantsToQuery(EntityType entityType) {
* @return List of tenants to query
*/
public List<String> getTenantsToQueryForColumnValues(EntityType entityType) {
return getTenants(entityType);
return getTenants(entityType, executionContext.getUserId());
}

private List<String> getTenants(EntityType entityType) {
private List<String> getTenants(EntityType entityType, UUID userId) {
log.info("Getting tenants to query for user {}", userId);
// Get the ECS tenant info first, since this comes from mod-users and should work in non-ECS environments
// We can use this for determining if it's an ECS environment, and if so, retrieving the consortium ID and central tenant ID
Map<String, String> ecsTenantInfo = getEcsTenantInfo();
Expand All @@ -76,18 +89,18 @@ private List<String> getTenants(EntityType entityType) {

List<String> tenantsToQuery = new ArrayList<>();
tenantsToQuery.add(centralTenantId);
List<Map<String, String>> userTenantMaps = getUserTenants(ecsTenantInfo.get("consortiumId"), executionContext.getUserId().toString());
List<Map<String, String>> userTenantMaps = getUserTenants(ecsTenantInfo.get("consortiumId"), userId.toString());
for (var userMap : userTenantMaps) {
String tenantId = userMap.get("tenantId");
String userId = userMap.get("userId");
String currentUserId = userMap.get("userId");
if (!tenantId.equals(centralTenantId)) {
try {
permissionsService.verifyUserHasNecessaryPermissions(tenantId, entityType, true);
permissionsService.verifyUserHasNecessaryPermissions(tenantId, entityType, UUID.fromString(currentUserId), true);
tenantsToQuery.add(tenantId);
} catch (MissingPermissionsException e) {
log.info("User with id {} does not have permissions to query tenant {}. Skipping.", userId, tenantId);
log.info("User with id {} does not have permissions to query tenant {}. Skipping.", currentUserId, tenantId);
} catch (FeignException e) {
log.error("Error retrieving permissions for user ID %s in tenant %s".formatted(userId, tenantId), e);
log.error("Error retrieving permissions for user ID %s in tenant %s".formatted(currentUserId, tenantId), e);
throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.folio.fqm.service;

import java.util.Set;
import java.util.UUID;

import lombok.extern.log4j.Log4j2;
import org.folio.querytool.domain.dto.EntityType;
import org.springframework.context.annotation.Lazy;
Expand All @@ -17,7 +19,7 @@ public Set<String> getUserPermissions() {
}

@Override
public Set<String> getUserPermissions(String tenantId) {
public Set<String> getUserPermissions(String tenantId, UUID userId) {
return Set.of();
}

Expand All @@ -36,11 +38,12 @@ public void verifyUserHasNecessaryPermissions(EntityType entityType, boolean che
}

@Override
public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, boolean checkFqmPermissions) {
public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, UUID userId, boolean checkFqmPermissions) {
log.info(
"Bypassing permissions check for tenantId: {}, entity type: {}, checkFqmPermissions={}",
"Bypassing permissions check for tenantId: {}, entity type: {}, userId: {}, checkFqmPermissions={}",
tenantId,
entityType.getName(),
userId,
checkFqmPermissions
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ public class PermissionsRegularService implements PermissionsService {

@Override
public Set<String> getUserPermissions() {
return getUserPermissions(context.getTenantId());
return getUserPermissions(context.getTenantId(), context.getUserId());
}

public Set<String> getUserPermissions(String tenantId) {
TenantUserPair key = new TenantUserPair(tenantId, context.getUserId());
public Set<String> getUserPermissions(String tenantId, UUID userId) {
TenantUserPair key = new TenantUserPair(tenantId, userId);
return cache.get(key, k -> isEureka ? getUserPermissionsFromRolesKeycloak(k.tenant(), k.userId()) : getUserPermissionsFromModPermissions(k.tenant(), k.userId()));
}

Expand All @@ -65,12 +65,12 @@ public Set<String> getRequiredPermissions(EntityType entityType) {

@Override
public void verifyUserHasNecessaryPermissions(EntityType entityType, boolean checkFqmPermissions) {
verifyUserHasNecessaryPermissions(context.getTenantId(), entityType, checkFqmPermissions);
verifyUserHasNecessaryPermissions(context.getTenantId(), entityType, context.getUserId(), checkFqmPermissions);
}

public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, boolean checkFqmPermissions) {
public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, UUID userId, boolean checkFqmPermissions) {
Set<String> requiredPermissions = getRequiredPermissions(entityType);
Set<String> userPermissions = getUserPermissions(tenantId);
Set<String> userPermissions = getUserPermissions(tenantId, userId);

Set<String> missingPermissions = new HashSet<>();
if (checkFqmPermissions) {
Expand All @@ -88,7 +88,7 @@ public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entity
}

if (!missingPermissions.isEmpty()) {
log.warn("User {} is missing permissions that are required for this operation: [{}]", context.getUserId(), missingPermissions);
log.warn("User {} is missing permissions that are required for this operation: [{}]", userId, missingPermissions);
throw new MissingPermissionsException(missingPermissions);
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/folio/fqm/service/PermissionsService.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
package org.folio.fqm.service;

import java.util.Set;
import java.util.UUID;

import org.folio.querytool.domain.dto.EntityType;

public interface PermissionsService {
public Set<String> getUserPermissions();

public Set<String> getUserPermissions(String tenantId);
public Set<String> getUserPermissions(String tenantId, UUID userId);

public Set<String> getRequiredPermissions(EntityType entityType);

public void verifyUserHasNecessaryPermissions(EntityType entityType, boolean checkFqmPermissions);

public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, boolean checkFqmPermissions);
public void verifyUserHasNecessaryPermissions(String tenantId, EntityType entityType, UUID userId, boolean checkFqmPermissions);
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,17 @@ public List<List<String>> getSortedIds(UUID queryId, int offset, int limit) {
return queryResultsSorterService.getSortedIds(queryId, offset, limit);
}

public List<Map<String, Object>> getContents(UUID entityTypeId, List<String> fields, List<List<String>> ids, boolean localize) {
public List<Map<String, Object>> getContents(UUID entityTypeId, List<String> fields, List<List<String>> ids, UUID userId, boolean localize, boolean privileged) {
EntityType entityType = entityTypeService.getEntityTypeDefinition(entityTypeId, true, false);
EntityTypeUtils.getIdColumnNames(entityType)
.forEach(colName -> {
if (!fields.contains(colName)) {
fields.add(colName);
}
});
List<String> tenantsToQuery = crossTenantQueryService.getTenantsToQuery(entityType);
List<String> tenantsToQuery = privileged
? crossTenantQueryService.getTenantsToQuery(entityType, userId)
: crossTenantQueryService.getTenantsToQuery(entityType);
return resultSetService.getResultSet(entityTypeId, fields, ids, tenantsToQuery, localize);
}

Expand Down
50 changes: 48 additions & 2 deletions src/test/java/org/folio/fqm/controller/FqlQueryControllerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ void shouldGetContents() throws Exception {
Map.of("id", id1, "field1", "value1", "field2", "value2"),
Map.of("id", id2, "field1", "value3", "field2", "value4")
);
when(queryManagementService.getContents(entityTypeId, fields, ids, false)).thenReturn(expectedContent);
when(queryManagementService.getContents(entityTypeId, fields, ids, null, false, false)).thenReturn(expectedContent);
RequestBuilder builder = post("/query/contents").contentType(MediaType.APPLICATION_JSON)
.header(XOkapiHeaders.TENANT, TENANT_ID)
.contentType(MediaType.APPLICATION_JSON)
Expand All @@ -337,6 +337,7 @@ void shouldGetContents() throws Exception {
.andExpect(jsonPath("$.[1].field2", is(expectedContent.get(1).get("field2"))));
}


@Test
void getContentsShouldReturn404WhenEntityTypeNotFound() throws Exception {
UUID entityTypeId = UUID.randomUUID();
Expand All @@ -350,7 +351,7 @@ void getContentsShouldReturn404WhenEntityTypeNotFound() throws Exception {
ContentsRequest contentsRequest = new ContentsRequest().entityTypeId(entityTypeId)
.fields(fields)
.ids(ids);
when(queryManagementService.getContents(entityTypeId, fields, ids, false)).thenThrow(new EntityTypeNotFoundException(entityTypeId));
when(queryManagementService.getContents(entityTypeId, fields, ids, null, false, false)).thenThrow(new EntityTypeNotFoundException(entityTypeId));
RequestBuilder builder = post("/query/contents").contentType(MediaType.APPLICATION_JSON)
.header(XOkapiHeaders.TENANT, TENANT_ID)
.contentType(MediaType.APPLICATION_JSON)
Expand Down Expand Up @@ -378,4 +379,49 @@ void getContentsShouldReturn400WhenParametersNotProvided() throws Exception {
.andExpect(status().isBadRequest());
}

@Test
void shouldGetContentsPrivileged() throws Exception {
UUID entityTypeId = UUID.randomUUID();
UUID userId = UUID.randomUUID();
UUID id1 = UUID.randomUUID();
List<String> fields = List.of("id");
List<List<String>> ids = List.of(
List.of(id1.toString())
);
ContentsRequest contentsRequest = new ContentsRequest().entityTypeId(entityTypeId)
.fields(fields)
.ids(ids)
.userId(userId);
List<Map<String, Object>> expectedContent = List.of(
Map.of("id", id1)
);
when(queryManagementService.getContents(entityTypeId, fields, ids, userId, false, true)).thenReturn(expectedContent);
RequestBuilder builder = post("/query/contents/privileged").contentType(MediaType.APPLICATION_JSON)
.header(XOkapiHeaders.TENANT, TENANT_ID)
.contentType(MediaType.APPLICATION_JSON)
.content(new ObjectMapper().writeValueAsString(contentsRequest));
mockMvc.perform(builder)
.andExpect(status().isOk())
.andExpect(jsonPath("$.[0].id", is(expectedContent.get(0).get("id").toString())));
}

@Test
void shouldReturn400WhenMissingUserIdForContentsPrivileged() throws Exception {
UUID entityTypeId = UUID.randomUUID();
UUID id1 = UUID.randomUUID();
List<String> fields = List.of("id");
List<List<String>> ids = List.of(
List.of(id1.toString())
);
ContentsRequest contentsRequest = new ContentsRequest().entityTypeId(entityTypeId)
.fields(fields)
.ids(ids);
RequestBuilder builder = post("/query/contents/privileged").contentType(MediaType.APPLICATION_JSON)
.header(XOkapiHeaders.TENANT, TENANT_ID)
.contentType(MediaType.APPLICATION_JSON)
.content(new ObjectMapper().writeValueAsString(contentsRequest));
mockMvc.perform(builder)
.andExpect(status().isBadRequest());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.mockito.junit.jupiter.MockitoExtension;

import java.util.List;
import java.util.Map;
import java.util.UUID;

import static org.junit.jupiter.api.Assertions.*;
Expand Down Expand Up @@ -165,13 +166,15 @@ void shouldAttemptCrossTenantQueryIfForceParamIsTrue() {
@Test
void shouldNotQueryTenantIfUserLacksTenantPermissions() {
String tenantId = "tenant_01";
UUID userId = UUID.fromString("a5e7895f-503c-4335-8828-f507bc8d1c45");
List<String> expectedTenants = List.of("tenant_01", "tenant_02");

when(executionContext.getTenantId()).thenReturn(tenantId);
when(executionContext.getUserId()).thenReturn(userId);
when(userTenantService.getUserTenantsResponse(tenantId)).thenReturn(ECS_TENANT_INFO);
when(ecsClient.get(eq("consortia/bdaa4720-5e11-4632-bc10-d4455cf252df/user-tenants"), anyMap())).thenReturn(USER_TENANT_JSON);
doNothing().when(permissionsService).verifyUserHasNecessaryPermissions("tenant_02", entityType, true);
doThrow(MissingPermissionsException.class).when(permissionsService).verifyUserHasNecessaryPermissions("tenant_03", entityType, true);
doNothing().when(permissionsService).verifyUserHasNecessaryPermissions("tenant_02", entityType, userId, true);
doThrow(MissingPermissionsException.class).when(permissionsService).verifyUserHasNecessaryPermissions("tenant_03", entityType, userId, true);
List<String> actualTenants = crossTenantQueryService.getTenantsToQuery(entityType);
assertEquals(expectedTenants, actualTenants);
}
Expand Down Expand Up @@ -204,6 +207,20 @@ void shouldGetListOfTenantsToQueryForColumnValues() {
assertEquals(expectedTenants, actualTenants);
}

@Test
void shouldGetListOfTenantsToQueryForSpecifiedUser() {
String tenantId = "tenant_01";
UUID userId = UUID.randomUUID();
List<String> expectedTenants = List.of("tenant_01", "tenant_02", "tenant_03");

when(executionContext.getTenantId()).thenReturn(tenantId);
when(userTenantService.getUserTenantsResponse(tenantId)).thenReturn(ECS_TENANT_INFO);
when(ecsClient.get("consortia/bdaa4720-5e11-4632-bc10-d4455cf252df/user-tenants", Map.of("userId", userId.toString(), "limit", "1000"))).thenReturn(USER_TENANT_JSON);

List<String> actualTenants = crossTenantQueryService.getTenantsToQuery(entityType, userId);
assertEquals(expectedTenants, actualTenants);
}

@Test
void shouldGetCentralTenantId() {
String expectedId = "tenant_01";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.folio.querytool.domain.dto.EntityType;
import org.junit.jupiter.api.Test;

import java.util.UUID;

class PermissionsBypassServiceTest {

// all methods should work with zero context
Expand All @@ -16,12 +18,12 @@ class PermissionsBypassServiceTest {
@Test
void testAllMethodsBypassed() {
assertThat(permissionsService.getUserPermissions(), is(empty()));
assertThat(permissionsService.getUserPermissions("foo"), is(empty()));
assertThat(permissionsService.getUserPermissions("foo", UUID.randomUUID()), is(empty()));
assertThat(permissionsService.getRequiredPermissions(new EntityType()), is(empty()));

assertDoesNotThrow(() -> {
permissionsService.verifyUserHasNecessaryPermissions("tenant_01", new EntityType(), false);
permissionsService.verifyUserHasNecessaryPermissions(new EntityType(), true);
permissionsService.verifyUserHasNecessaryPermissions("tenant_01", new EntityType(), UUID.randomUUID(),true);
});
}
}
Loading

0 comments on commit 1a6b13b

Please sign in to comment.