Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fixes in building ResourceAccessManager's securityFilter #74

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Bug fixes in building ResourceAccessManager's securityFilter #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions ...cation/authorization-api/src/main/java/org/geoserver/acl/authorization/AccessSummary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
*/
package org.geoserver.acl.authorization;

import static org.geoserver.acl.authorization.WorkspaceAccessSummary.ANY;
import static org.geoserver.acl.authorization.WorkspaceAccessSummary.NO_WORKSPACE;

import static java.lang.String.format;

import lombok.EqualsAndHashCode;
Expand All @@ -17,6 +20,7 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.stream.Collectors;

/**
Expand All @@ -29,16 +33,16 @@
@EqualsAndHashCode
public class AccessSummary {

private static final WorkspaceAccessSummary HIDDEN =
/** Immutable mapping of workspace name to summary */
private Map<String, WorkspaceAccessSummary> workspaceSummaries;

private static final WorkspaceAccessSummary HIDE_ALL =
WorkspaceAccessSummary.builder()
.workspace("*")
.adminAccess(null)
.addForbidden("*")
.build();

/** Immutable mapping of workspace name to summary */
private Map<String, WorkspaceAccessSummary> workspaceSummaries;

private AccessSummary(Map<String, WorkspaceAccessSummary> workspaceSummaries) {
this.workspaceSummaries = workspaceSummaries;
}
Expand All @@ -62,13 +66,13 @@ public WorkspaceAccessSummary workspace(String workspace) {
}

public boolean hasAdminReadAccess(@NonNull String workspaceName) {
boolean user = workspaceSummaries.getOrDefault("*", HIDDEN).isUser();
return user ? user : workspaceSummaries.getOrDefault(workspaceName, HIDDEN).isUser();
boolean user = workspaceSummaries.getOrDefault(ANY, HIDE_ALL).isUser();
return user ? user : workspaceSummaries.getOrDefault(workspaceName, HIDE_ALL).isUser();
}

public boolean hasAdminWriteAccess(@NonNull String workspaceName) {
boolean admin = workspaceSummaries.getOrDefault("*", HIDDEN).isAdmin();
return admin ? admin : workspaceSummaries.getOrDefault(workspaceName, HIDDEN).isAdmin();
boolean admin = workspaceSummaries.getOrDefault(ANY, HIDE_ALL).isAdmin();
return admin ? admin : workspaceSummaries.getOrDefault(workspaceName, HIDE_ALL).isAdmin();
}

public boolean canSeeLayer(String workspaceName, @NonNull String layerName) {
Expand All @@ -79,12 +83,16 @@ public boolean canSeeLayer(String workspaceName, @NonNull String layerName) {

private WorkspaceAccessSummary summary(@NonNull String workspaceName) {
var summary = workspaceSummaries.get(workspaceName);
if (null == summary) summary = workspaceSummaries.getOrDefault("*", HIDDEN);
if (null == summary) summary = workspaceSummaries.getOrDefault(ANY, HIDE_ALL);
return summary;
}

public Set<String> visibleWorkspaces() {
return workspaceSummaries.keySet();
return workspaceSummaries.values().stream()
.filter(WorkspaceAccessSummary::visible)
.map(WorkspaceAccessSummary::getWorkspace)
.filter(name -> !NO_WORKSPACE.equals(name))
.collect(Collectors.toCollection(TreeSet::new));
}

public Set<String> adminableWorkspaces() {
Expand All @@ -96,7 +104,7 @@ public Set<String> adminableWorkspaces() {
@Override
public String toString() {
var values = new TreeMap<>(workspaceSummaries).values();
return format("%s[%s]", getClass().getSimpleName(), values);
return format("%s(%s)", getClass().getSimpleName(), values);
}

public boolean hasAdminRightsToAnyWorkspace() {
Expand Down
11 changes: 10 additions & 1 deletion ...thorization-api/src/main/java/org/geoserver/acl/authorization/WorkspaceAccessSummary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
@Value
@Builder(builderClassName = "Builder")
public class WorkspaceAccessSummary implements Comparable<WorkspaceAccessSummary> {

public static final String NO_WORKSPACE = "";
public static final String ANY = "*";

Expand Down Expand Up @@ -58,10 +59,18 @@ public class WorkspaceAccessSummary implements Comparable<WorkspaceAccessSummary
*/
@NonNull private Set<String> forbidden;

public boolean hideAll() {
return getAllowed().isEmpty() && getForbidden().contains(ANY);
}

public boolean visible() {
return !hideAll();
}

@Override
public String toString() {
return format(
"[%s: admin: %s, allowed: %s, forbidden: %s]",
"workapce: %s: admin: %s, allowed: %s, forbidden: %s",
workspace, adminAccess, allowed, forbidden);
}

Expand Down
71 changes: 46 additions & 25 deletions ...er/src/main/java/org/geoserver/acl/plugin/accessmanager/CatalogSecurityFilterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@

import static org.geoserver.acl.authorization.WorkspaceAccessSummary.ANY;
import static org.geoserver.acl.authorization.WorkspaceAccessSummary.NO_WORKSPACE;
import static org.geoserver.catalog.Predicates.*;
import static org.geoserver.catalog.Predicates.acceptAll;
import static org.geoserver.catalog.Predicates.acceptNone;
import static org.geoserver.catalog.Predicates.and;
import static org.geoserver.catalog.Predicates.equal;
import static org.geoserver.catalog.Predicates.in;
import static org.geoserver.catalog.Predicates.isInstanceOf;
import static org.geoserver.catalog.Predicates.isNull;
import static org.geoserver.catalog.Predicates.not;
import static org.geoserver.catalog.Predicates.notEqual;
import static org.geoserver.catalog.Predicates.or;
import static org.geotools.api.filter.Filter.EXCLUDE;
import static org.geotools.api.filter.Filter.INCLUDE;
Expand All @@ -35,7 +37,6 @@
import org.springframework.lang.NonNull;
import org.springframework.util.Assert;

import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -64,13 +65,13 @@ public Filter build(Class<? extends CatalogInfo> clazz) {
return EXCLUDE;
}
if (WorkspaceInfo.class.isAssignableFrom(clazz)) {
return workspaceNameFilter("name");
return workspaceNameFilter("name", false);
}
if (NamespaceInfo.class.isAssignableFrom(clazz)) {
return workspaceNameFilter("prefix");
return workspaceNameFilter("prefix", false);
}
if (StoreInfo.class.isAssignableFrom(clazz)) {
return workspaceNameFilter("workspace.name");
return workspaceNameFilter("workspace.name", false);
}
if (ResourceInfo.class.isAssignableFrom(clazz)) {
return layerFilter("store.workspace.name", "name", ResourceInfo.class);
Expand All @@ -86,7 +87,7 @@ public Filter build(Class<? extends CatalogInfo> clazz) {
}

private Filter styleFilter() {
return workspaceNameFilter("workspace.name");
return workspaceNameFilter("workspace.name", true);
}

private Filter publishedInfoFilter(Class<? extends PublishedInfo> clazz) {
Expand All @@ -98,9 +99,11 @@ private Filter publishedInfoFilter(Class<? extends PublishedInfo> clazz) {
}
Filter layerInfoFilter = build(LayerInfo.class);
Filter layerGroupInfoFilter = build(LayerGroupInfo.class);

Filter layerFilter = and(isInstanceOf(LayerInfo.class), layerInfoFilter);
Filter groupFilter = and(isInstanceOf(LayerGroupInfo.class), layerGroupInfoFilter);
if (INCLUDE.equals(layerInfoFilter) && INCLUDE.equals(layerGroupInfoFilter)) {
return INCLUDE;
}
Filter layerFilter = instanceOfAnd(LayerInfo.class, layerInfoFilter);
Filter groupFilter = instanceOfAnd(LayerGroupInfo.class, layerGroupInfoFilter);

if (EXCLUDE.equals(layerInfoFilter)) {
return groupFilter;
Expand All @@ -111,6 +114,11 @@ private Filter publishedInfoFilter(Class<? extends PublishedInfo> clazz) {
return or(layerFilter, groupFilter);
}

private Filter instanceOfAnd(Class<?> typeOf, Filter andThen) {
if (EXCLUDE.equals(andThen)) return andThen;
return and(isInstanceOf(typeOf), andThen);
}

private Filter layerFilter(
String workspaceProperty, String nameProperty, Class<? extends CatalogInfo> type) {
List<WorkspaceAccessSummary> summaries = viewables.getWorkspaces();
Expand All @@ -119,15 +127,21 @@ private Filter layerFilter(
Set<String> hideAllWorkspaceNames = new TreeSet<>();
for (WorkspaceAccessSummary wsSummary : summaries) {
String workspace = wsSummary.getWorkspace();
if (isHideAll(wsSummary)) {
hideAllWorkspaceNames.add(workspace);
if (wsSummary.hideAll()) {
if (!ANY.equals(workspace)) {
hideAllWorkspaceNames.add(workspace);
}
} else {
boolean isNullWorkspace = NO_WORKSPACE.equals(workspace);
boolean supportsNullWorkspace = LayerGroupInfo.class.equals(type);
// ignore if workspace is null and type is LayerInfo or ResourceInfo
if (!isNullWorkspace || supportsNullWorkspace) {
Filter wsLayersFitler =
filterLayersOnWorkspace(wsSummary, workspaceProperty, nameProperty);
filterLayersOnWorkspace(
wsSummary,
workspaceProperty,
supportsNullWorkspace,
nameProperty);

if (EXCLUDE.equals(filter)) {
filter = wsLayersFitler;
Expand Down Expand Up @@ -159,19 +173,19 @@ private Filter denyWorkspacesFilter(
return notEqualOrIn(workspaceProperty, hideAllWorkspaceNames, EXCLUDE);
}

private boolean isHideAll(WorkspaceAccessSummary ws) {
return ws.getAllowed().isEmpty() && ws.getForbidden().contains(ANY);
}

@NonNull
private Filter filterLayersOnWorkspace(
WorkspaceAccessSummary vl, String workspaceProperty, String nameProperty) {
WorkspaceAccessSummary vl,
String workspaceProperty,
boolean includeNullWorkspace,
String nameProperty) {

final String workspace = vl.getWorkspace();
final Set<String> allowed = vl.getAllowed();
final Set<String> forbidden = vl.getForbidden();

Filter workspaceFilter = workspaceNameFilter(workspaceProperty, Set.of(workspace));
Filter workspaceFilter =
workspaceNameFilter(workspaceProperty, includeNullWorkspace, Set.of(workspace));
Filter filter;
if (allowed.isEmpty() && forbidden.isEmpty()) {
filter = workspaceFilter;
Expand Down Expand Up @@ -227,19 +241,26 @@ private Set<String> getVisibleWorkspaces() {
return viewables.visibleWorkspaces();
}

private Filter workspaceNameFilter(String workspaceProperty) {
private Filter workspaceNameFilter(String workspaceProperty, boolean includeNullWorkspace) {
Set<String> visibleWorkspaces = getVisibleWorkspaces();
return workspaceNameFilter(workspaceProperty, visibleWorkspaces);
if (includeNullWorkspace && viewables.workspace(NO_WORKSPACE) != null) {
visibleWorkspaces = new TreeSet<>(visibleWorkspaces);
visibleWorkspaces.add(NO_WORKSPACE);
}
return workspaceNameFilter(workspaceProperty, includeNullWorkspace, visibleWorkspaces);
}

private Filter workspaceNameFilter(String workspaceProperty, Set<String> visibleWorkspaces) {
private Filter workspaceNameFilter(
String workspaceProperty, boolean includeNullWorkspace, Set<String> visibleWorkspaces) {
if (visibleWorkspaces.contains(ANY)) {
return acceptAll();
}
Filter filter = acceptAll();
Filter filter = acceptNone();
if (visibleWorkspaces.contains(NO_WORKSPACE)) {
filter = isNull(workspaceProperty);
visibleWorkspaces = new HashSet<>(visibleWorkspaces);
if (includeNullWorkspace) {
filter = isNull(workspaceProperty);
}
visibleWorkspaces = new TreeSet<>(visibleWorkspaces);
visibleWorkspaces.remove(NO_WORKSPACE);
}
if (!visibleWorkspaces.isEmpty()) {
Expand All @@ -250,7 +271,7 @@ private Filter workspaceNameFilter(String workspaceProperty, Set<String> visible
} else {
namesFilter = in(workspaceProperty, workspaces);
}
if (INCLUDE.equals(filter)) {
if (EXCLUDE.equals(filter)) {
filter = namesFilter;
} else {
filter = or(filter, namesFilter);
Expand Down
Loading
Loading