Homogenize permissions checks #215
Conversation
…neffective on count
Signed-off-by: David BRAQUART <[email protected]>
| DirectoryElementRepository.ElementParentage::getParentId, | ||
| Collectors.counting() | ||
| )); | ||
| } |
There was a problem hiding this comment.
I think we can do differently: rather than finding all children in N directories (each child will be checked by checking its parent directory), we could first eliminate the non-readable dirs among the N, and then retrieve the children ?
Ex:
List<UUID> readableSubDirectories = subDirectories.stream().filter(dirId -> checkPermission(userId, List.of(dirId), READ)).toList();
return repositoryService.findAllByParentIdInAndTypeIn(readableSubDirectories, types).stream()
.collect(Collectors.groupingBy(
DirectoryElementRepository.ElementParentage::getParentId,
Collectors.counting()
));
There was a problem hiding this comment.
In our current uses cases it might be a bit overkill because we already check directories permissions in functions calling getSubDirectoriesCounts but I reckon it's still worth because by adding it here too we're avoiding future permissions mistakes related to it if it gets involved in new use cases
…ubDirectoriesCounts
|
| return repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() | ||
| List<UUID> readableSubDirectories = subDirectories.stream().filter(dirId -> hasReadPermissions(userId, List.of(dirId))).toList(); | ||
| return repositoryService.findAllByParentIdInAndTypeIn(readableSubDirectories, types).stream() | ||
| .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) |
There was a problem hiding this comment.
All children are in readable dirs, so we dont need to check (their parent dir) anymore.
Or maybe we have to filter/check in case the child is a sub-dir ?
There was a problem hiding this comment.
Ye a subdirectory can be read protected while its parent isn't so I don't think we can bypass this check
2 participants
No description provided.