Skip to content

Commit

Permalink
Merge pull request #74 from groldan/fix_security_filter
Browse files Browse the repository at this point in the history 
Bug fixes in building ResourceAccessManager's securityFilter
  • Loading branch information
@groldan
groldan authored Aug 13, 2024
2 parents 89474d9 + 1bee573 commit f72f6f8
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 51 deletions.
30 changes: 19 additions & 11 deletions ...cation/authorization-api/src/main/java/org/geoserver/acl/authorization/AccessSummary.java
View file
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
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
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

0 comments on commit f72f6f8

Please sign in to comment.