Skip to content

Commit

Permalink
Revised unadvertised resources filtering. (#405)
Browse files Browse the repository at this point in the history
  • Loading branch information
axl8713 authored Feb 3, 2025
1 parent b1f630d commit 87ac2a9
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public void addAdvertisedSecurityConstraints(Search searchCriteria, User user) {
Filter userFiltering = Filter.equal("user.name", user.getName());

// Combine owner and advertisedFilter using OR
/** The user is the owner of the resource or the resource is advertised. */
/* The user is the owner of the resource or the resource is advertised. */
Filter advertisedFiltering =
Filter.or(
Filter.equal("user.name", user.getName()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,6 @@
import it.geosolutions.geostore.core.model.User;

public interface ResourcePermissionService {
/**
* This method allows us to know if we filter out "unadvertised" resources for
* non-admin/non-owners, keeping only owned resources.
*
* <p>Be aware to fetch the user security rules prior to call this method.
*
* @param resource
* @param user
* @return <code>true</code> if the resource should be visible to the user, <code>false</code>
* otherwise
* @throws IllegalArgumentException if the user security rules have not been initialized
* properly
*/
boolean isResourceAvailableForUser(Resource resource, User user);

/**
* Check if the user has at least one {@link it.geosolutions.geostore.core.model.SecurityRule}
* associated in which he is the user.
*
* <p>Be aware to fetch the user security rules prior to call this method.
*
* @param user
* @param resource
* @return @return <code>true</code> if the user is the owner of the resource, <code>false
* </code> otherwise
* @throws IllegalArgumentException if the user security rules have not been initialized
* properly
*/
boolean isUserOwner(User user, Resource resource);

/**
* Verifies whether the user or any of their groups is the owner of the resource and has read
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,6 @@ public class ResourcePermissionServiceImpl implements ResourcePermissionService
resourceOwnershipWithWritePermission.apply(rule, resource)
&& resourceOwnershipWithReadPermission.apply(rule, resource);

@Override
public boolean isResourceAvailableForUser(Resource resource, User user) {
return resource.isAdvertised()
|| user.getRole().equals(Role.ADMIN)
|| isUserOwner(user, resource);
}

@Override
public boolean isUserOwner(User user, Resource resource) {
checkUserSecurityRules(user);
return user.getSecurity().stream()
.anyMatch(rule -> resourceOwnership.apply(rule, resource));
}

@Override
public boolean canUserReadResource(User user, Long resourceId) {
Resource resource = new Resource();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ private List<Resource> searchResources(ResourceSearchParameters parameters)
searchCriteria.addFetch("security");
searchCriteria.setDistinct(true);

securityDAO.addReadSecurityConstraints(searchCriteria, parameters.getAuthUser());
securityDAO.addAdvertisedSecurityConstraints(searchCriteria, parameters.getAuthUser());
return this.search(searchCriteria);
}

Expand All @@ -703,7 +703,6 @@ private List<ShortResource> convertToShortResourceList(List<Resource> resources,
userService.fetchSecurityRules(user);

return resources.stream()
.filter(r -> resourcePermissionService.isResourceAvailableForUser(r, user))
.map(r -> createShortResource(user, r))
.collect(Collectors.toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,15 @@ public String getResourcesByCategory(
boolean shouldIncludeAttributes =
includeAttributes || (extraAttributes != null && !extraAttributes.isEmpty());
List<Resource> resources =
filterOutUnavailableResources(
resourceService.getResources(
ResourceSearchParameters.builder()
.filter(filter)
.page(page)
.entries(limit)
.includeAttributes(shouldIncludeAttributes)
.includeData(includeData)
.authUser(authUser)
.build()),
authUser);
resourceService.getResources(
ResourceSearchParameters.builder()
.filter(filter)
.page(page)
.entries(limit)
.includeAttributes(shouldIncludeAttributes)
.includeData(includeData)
.authUser(authUser)
.build());

long count = 0;
if (!resources.isEmpty()) {
Expand Down Expand Up @@ -362,19 +360,17 @@ public ExtResourceList getExtResourcesList(

try {
List<Resource> resources =
filterOutUnavailableResources(
resourceService.getResources(
ResourceSearchParameters.builder()
.filter(filter)
.page(page)
.entries(limit)
.sortBy(sort.getSortBy())
.sortOrder(sort.getSortOrder())
.includeAttributes(includeAttributes)
.includeData(includeData)
.authUser(authUser)
.build()),
authUser);
resourceService.getResources(
ResourceSearchParameters.builder()
.filter(filter)
.page(page)
.entries(limit)
.sortBy(sort.getSortBy())
.sortOrder(sort.getSortOrder())
.includeAttributes(includeAttributes)
.includeData(includeData)
.authUser(authUser)
.build());

long count = 0;
if (!resources.isEmpty()) {
Expand All @@ -390,15 +386,6 @@ public ExtResourceList getExtResourcesList(
}
}

private List<Resource> filterOutUnavailableResources(List<Resource> resources, User user) {

userService.fetchSecurityRules(user);

return resources.stream()
.filter(r -> resourcePermissionService.isResourceAvailableForUser(r, user))
.collect(Collectors.toList());
}

/**
* Filters out unadvertised resources and adds permission information to the resources.
*
Expand Down Expand Up @@ -757,6 +744,9 @@ private void readSecurity() {
canEdit = sr.isCanEdit();
return;
}

userService.fetchSecurityRules(authUser);

if (authUser != null && resourcePermissionService.canUserWriteResource(authUser, r)) {
canEdit = true;
canDelete = true;
Expand Down

0 comments on commit 87ac2a9

Please sign in to comment.