[GEOS-11399] Use Catalog streaming API in LayerGroupPage

[groldan]

Make webui's LayerGroupProvider use streaming catalog access

Checklist

• I have read the contribution guidelines.
• I have sent a Contribution Licence Agreement (not required for small changes, e.g., fixing typos in documentation).
• First PR targets the main branch (backports managed later; ignore for branch specific issues).
• All the build checks are green (see automated QA checks).

For core and extension modules:

• New unit tests have been added covering the changes.
• Documentation has been updated (if change is visible to end users).
• The REST API docs have been updated (when changing configuration objects or the REST controllers).
• There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
• Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
• Bug fixes and small new features are presented as a single commit.
• Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

[groldan]

@aaime help

In changing LayerGroupProvider to use the catalog streaming API instead of getLayerGroups():List<>,
AbstractAdminPrivilegeTest.testLayerGroupAllPage() is failing.

Turns out Catalog.getLayerGroups() and Catalog.list(LayerGroupInfo.class, Filter.INCLUDE) return different results.
Logged in as the cite test user, the former includes the global layer groups, but the later doesn't, and only includes the cite workspace's cite_local group.

For instance, one would expect the following to pass:

        AdminRequest.start(new Object());
        Set<String> withGet = cat.getLayerGroups().stream().map(LayerGroupInfo::getName).collect(Collectors.toCollection(TreeSet::new));

        AdminRequest.start(new Object());
        Set<String> withList = new TreeSet<>();
        try(CloseableIterator<LayerGroupInfo> it = cat.list(LayerGroupInfo.class, Filter.INCLUDE)){
        	while(it.hasNext()) withList.add(it.next().getName());
        }
        assertEquals(withGet, withList);

But we get this instead:

java.lang.AssertionError: expected:<[cite_global, cite_local, sf_global]> but was:<[cite_local]>

When calling Catalog.list(LayerGroupInfo.class, Filter.INCLUDE), SecureCatalogImpl calls CatalogFacade.list() methods is called with the following filter:

[Filter.INCLUDE AND [ workspace.name = cite ] AND Filter.INCLUDE AND [ true = InternalFunctionImpl() ] AND Filter.INCLUDE]

But when calling Catalog.getLayerGroups(), such filter is never built. SecureCatalogImpl instead gets all layer groups and filters the list with

protected LayerGroupInfo checkAccess(
            Authentication user,
            LayerGroupInfo group,
            MixedModeBehavior mixedModeBehavior,
            List<LayerGroupInfo> containers)

which includes the global groups.

Now the question is which one is right, first. And whether this exposes a bug or am I missing something?

TIA.

[aaime]

Scratch scratch... here are some quick thoughts.

The class is abstract and lets subclasses setup the security restrictions:

• The AdminPrivilegeTest sets up "cite" as the workspace administrator of the same named workspace, while general read access is granted to all.
• ResourceAccessManagerAdminPrivilegeTest seems to set up a test resource access manager that would turn cite into a workspace admin as well, not sure what happens to general requests.

In terms of what the secured catalog would have to do, the answer is contextual:

• If the current request is an admin request (e.g., Wicket admin GUI or REST API) then it should only return the layer groups the user can administer. So the answer would be, I think, only the groups in the cite workspace.
• If the current request is a general OGC request, then the rules for general data access apply
  • if the request is using a local workspace, then only current workspace groups should be returned, I believe
  • If the request is on the global service instead, then all layer groups would have to be returned.

That's what I believe the server should do... but mind, I've just jotted down some thoughts, could not invest time for a through investigation.