Skip to content

Commit

Permalink
Polish security enforcer
Browse files Browse the repository at this point in the history 
No functional changes, only safe code simplifications.
Work in progress.
  • Loading branch information
@mederly
mederly committed May 4, 2023
1 parent 6707aac commit 9530e45
Show file tree
Hide file tree
Showing 44 changed files with 2,753 additions and 2,282 deletions.
3 changes: 1 addition & 2 deletions ...ui/src/main/java/com/evolveum/midpoint/web/page/admin/resources/ResourceContentPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,8 +679,7 @@ protected ObjectQuery createQuery() {
if (objectClass == null) {
return null;
}
return ObjectQueryUtil.createResourceAndObjectClassQuery(resourceModel.getObject().getOid(),
objectClass, getPageBase().getPrismContext());
return ObjectQueryUtil.createResourceAndObjectClassQuery(resourceModel.getObject().getOid(), objectClass);
}

ResourceObjectDefinition rOcDef = getDefinitionByKind();
Expand Down
4 changes: 2 additions & 2 deletions ...ui/src/main/java/com/evolveum/midpoint/web/page/admin/workflow/PageAttorneySelection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ private ObjectQuery getAttorneySelectionQuery() {
try {
ObjectQuery query = PageAttorneySelection.this.getPrismContext().queryFactory().createQuery();
// todo target authorization action
ObjectFilter filter = service.getDonorFilter(UserType.class, null, null,
task, task.getResult());
ObjectFilter filter = service.getDonorFilter(
UserType.class, null, null, task, task.getResult());

query.addFilter(filter);
return query;
Expand Down
22 changes: 14 additions & 8 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/AccessDecision.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,21 @@ public AuthorizationDecisionType getAuthorizationDecisionType() {
return authorizationDecisionType;
}

/**
* Rules:
*
* - `null` values are ignored,
* - any {@link AccessDecision#DENY} means {@link AccessDecision#DENY},
* - any {@link AccessDecision#DEFAULT} means {@link AccessDecision#DEFAULT},
* - otherwise, only {@link AccessDecision#ALLOW} remains, leading to {@link AccessDecision#ALLOW}.
*
* This also means that this operation is associative.
*/
public static AccessDecision combine(AccessDecision oldDecision, AccessDecision newDecision) {
if (oldDecision == null && newDecision == null) {
return null;
}
if (oldDecision == null && newDecision != null) {
if (oldDecision == null) {
return newDecision;
}
if (oldDecision != null && newDecision == null) {
if (newDecision == null) {
return oldDecision;
}
if (oldDecision == DENY || newDecision == DENY) {
Expand All @@ -60,7 +67,7 @@ public static AccessDecision combine(AccessDecision oldDecision, AccessDecision
if (oldDecision == ALLOW || newDecision == ALLOW) {
return ALLOW;
}
throw new IllegalStateException("Unexpected combine "+oldDecision+"+"+newDecision);
throw new IllegalStateException("Unexpected combine " + oldDecision + "+" + newDecision);
}

public static AccessDecision translate(AuthorizationDecisionType authorizationDecisionType) {
Expand All @@ -73,8 +80,7 @@ public static AccessDecision translate(AuthorizationDecisionType authorizationDe
case DENY:
return DENY;
default:
throw new IllegalStateException("Unknown AuthorizationDecisionType "+authorizationDecisionType);
throw new IllegalStateException("Unknown AuthorizationDecisionType " + authorizationDecisionType);
}
}

}
48 changes: 20 additions & 28 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,7 @@ public static String dump(QueryType query, @NotNull PrismContext prismContext) t
* *Please note: If provided `origFilter` is {@link AndFilter} it will be modified!*
* TODO consider moving to QueryFactory
*/
public static ObjectFilter filterAnd(ObjectFilter origFilter, ObjectFilter additionalFilter,
PrismContext prismContext) {
public static ObjectFilter filterAnd(ObjectFilter origFilter, ObjectFilter additionalFilter) {
if (origFilter == additionalFilter) {
// AND with itself
return origFilter;
Expand Down Expand Up @@ -284,7 +283,7 @@ public static ObjectFilter filterAnd(ObjectFilter origFilter, ObjectFilter addit
}
return origFilter;
}
return prismContext.queryFactory().createAnd(origFilter, additionalFilter);
return PrismContext.get().queryFactory().createAnd(origFilter, additionalFilter);
}

/**
Expand Down Expand Up @@ -336,8 +335,7 @@ public static ObjectFilter filterAndImmutable(
* *Please note: If provided `origFilter` is {@link OrFilter} it will be modified!*
* TODO consider moving to QueryFactory
*/
public static ObjectFilter filterOr(ObjectFilter origFilter, ObjectFilter additionalFilter,
PrismContext prismContext) {
public static ObjectFilter filterOr(ObjectFilter origFilter, ObjectFilter additionalFilter) {
if (origFilter == additionalFilter) {
// OR with itself
return origFilter;
Expand Down Expand Up @@ -366,7 +364,7 @@ public static ObjectFilter filterOr(ObjectFilter origFilter, ObjectFilter additi
}
return origFilter;
}
return prismContext.queryFactory().createOr(origFilter, additionalFilter);
return PrismContext.get().queryFactory().createOr(origFilter, additionalFilter);
}

/**
Expand Down Expand Up @@ -425,12 +423,6 @@ public static boolean isNone(ObjectFilter filter) {
* This always returns cloned filter which can be freely modify later.
*/
public static ObjectFilter simplify(ObjectFilter filter) {
return simplify(filter, PrismContext.get());
}

/** DEPRECATED - replace with simplify(filter) and make private when not used. */
@Deprecated
public static ObjectFilter simplify(ObjectFilter filter, PrismContext prismContext) {
if (filter == null) {
return null;
}
Expand All @@ -440,18 +432,18 @@ public static ObjectFilter simplify(ObjectFilter filter, PrismContext prismConte
for (ObjectFilter subfilter : conditions) {
if (subfilter instanceof NoneFilter) {
// AND with "false"
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else if (subfilter instanceof AllFilter || subfilter instanceof UndefinedFilter) {
// AND with "true", just skip it
} else {
ObjectFilter simplifiedSubfilter = simplify(subfilter, prismContext);
ObjectFilter simplifiedSubfilter = simplify(subfilter);
if (simplifiedSubfilter instanceof AndFilter) {
// Unwrap AND filter to parent and
for (ObjectFilter condition : ((AndFilter) simplifiedSubfilter).getConditions()) {
simplifiedFilter.addCondition(condition);
}
} else if (simplifiedSubfilter instanceof NoneFilter) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else if (simplifiedSubfilter == null || simplifiedSubfilter instanceof AllFilter) {
// skip
} else {
Expand All @@ -460,7 +452,7 @@ public static ObjectFilter simplify(ObjectFilter filter, PrismContext prismConte
}
}
if (simplifiedFilter.isEmpty()) {
return FilterCreationUtil.createAll(prismContext);
return FilterCreationUtil.createAll();
} else if (simplifiedFilter.getConditions().size() == 1) {
return simplifiedFilter.getConditions().iterator().next();
} else {
Expand All @@ -474,55 +466,55 @@ public static ObjectFilter simplify(ObjectFilter filter, PrismContext prismConte
// OR with "false", just skip it
} else if (subfilter instanceof AllFilter) {
// OR with "true"
return FilterCreationUtil.createAll(prismContext);
return FilterCreationUtil.createAll();
} else {
ObjectFilter simplifiedSubfilter = simplify(subfilter, prismContext);
ObjectFilter simplifiedSubfilter = simplify(subfilter);
if (simplifiedSubfilter instanceof NoneFilter) {
// skip
} else if (simplifiedSubfilter == null || simplifiedSubfilter instanceof AllFilter) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else {
simplifiedFilter.addCondition(simplifiedSubfilter);
}
}
}
if (simplifiedFilter.isEmpty()) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else if (simplifiedFilter.getConditions().size() == 1) {
return simplifiedFilter.getConditions().iterator().next();
} else {
return simplifiedFilter;
}
} else if (filter instanceof NotFilter) {
ObjectFilter subfilter = ((NotFilter) filter).getFilter();
ObjectFilter simplifiedSubfilter = simplify(subfilter, prismContext);
ObjectFilter simplifiedSubfilter = simplify(subfilter);
if (simplifiedSubfilter instanceof NoneFilter) {
return FilterCreationUtil.createAll(prismContext);
return FilterCreationUtil.createAll();
} else if (simplifiedSubfilter == null || simplifiedSubfilter instanceof AllFilter) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else {
NotFilter simplifiedFilter = ((NotFilter) filter).cloneEmpty();
simplifiedFilter.setFilter(simplifiedSubfilter);
return simplifiedFilter;
}
} else if (filter instanceof TypeFilter) {
ObjectFilter subFilter = ((TypeFilter) filter).getFilter();
ObjectFilter simplifiedSubfilter = simplify(subFilter, prismContext);
ObjectFilter simplifiedSubfilter = simplify(subFilter);
if (simplifiedSubfilter instanceof AllFilter) {
simplifiedSubfilter = null;
} else if (simplifiedSubfilter instanceof NoneFilter) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
}
TypeFilter simplifiedFilter = ((TypeFilter) filter).cloneEmpty();
simplifiedFilter.setFilter(simplifiedSubfilter);
return simplifiedFilter;
} else if (filter instanceof ExistsFilter) {
ObjectFilter subFilter = ((ExistsFilter) filter).getFilter();
ObjectFilter simplifiedSubfilter = simplify(subFilter, prismContext);
ObjectFilter simplifiedSubfilter = simplify(subFilter);
if (simplifiedSubfilter instanceof AllFilter) {
simplifiedSubfilter = null;
} else if (simplifiedSubfilter instanceof NoneFilter) {
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
}
ExistsFilter simplifiedFilter = ((ExistsFilter) filter).cloneEmpty();
simplifiedFilter.setFilter(simplifiedSubfilter);
Expand All @@ -537,7 +529,7 @@ public static ObjectFilter simplify(ObjectFilter filter, PrismContext prismConte
// As a general rule we can assume that these filters would always yield zero records
// so they can be replaced by None filter. Should this assumption turn out to be invalid,
// remove this optimization and implement correct behavior in repo query interpreter.
return FilterCreationUtil.createNone(prismContext);
return FilterCreationUtil.createNone();
} else {
return filter.clone();
}
Expand Down

0 comments on commit 9530e45

Please sign in to comment.