Skip to content

Commit

Permalink
Fix handling of NONE filter in ModelController
Browse files Browse the repository at this point in the history 
It was assumed that a filter (for search/count operations)
can be NONE only because of authorizations. This is not true:
the client can send such a filter as well.

This commit handles such a case correctly.
  • Loading branch information
@mederly @KaterynaHonchar
mederly authored and KaterynaHonchar committed Nov 23, 2023
1 parent dff89de commit 0d70b1e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 49 deletions.
5 changes: 5 additions & 0 deletions infra/schema/src/main/java/com/evolveum/midpoint/schema/SearchResultList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ public class SearchResultList<T> extends AbstractFreezable
private List<T> list = null;
private SearchResultMetadata metadata = null;

/** Returns modifiable instance, just to keep the existing behavior. */
public static <T> SearchResultList<T> empty() {
return new SearchResultList<>(new ArrayList<>());
}

@Override
protected void performFreeze() {
if (isMutable()) {
Expand Down
126 changes: 78 additions & 48 deletions ...model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,9 +561,12 @@ public <T extends ObjectType> SearchResultList<PrismObject<T>> searchObjects(
ObjectManager searchProvider = getObjectManager(type, options);
result.addArbitraryObjectAsParam("searchProvider", searchProvider);

if (checkNoneFilterBeforeAutz(query)) {
return SearchResultList.empty();
}
ObjectQuery processedQuery = preProcessQuerySecurity(type, query, rootOptions, task, result);
if (isFilterNone(processedQuery, result)) {
return new SearchResultList<>(new ArrayList<>());
if (checkNoneFilterAfterAutz(processedQuery, result)) {
return SearchResultList.empty();
}

enterModelMethod(); // outside try-catch because if this ends with an exception, cache is not entered yet
Expand Down Expand Up @@ -744,14 +747,18 @@ public <T extends Containerable> SearchResultList<T> searchContainers(
var parsedOptions = preProcessOptionsSecurity(rawOptionsReadWrite, task, result);
var options = parsedOptions.getCollection();

if (checkNoneFilterBeforeAutz(origQuery)) {
return SearchResultList.empty();
}

var ctx = new ContainerSearchLikeOpContext<>(type, origQuery, parsedOptions, task, result);

GetOperationOptions rootOptions = parsedOptions.getRootOptions();

ObjectQuery query = ctx.securityRestrictedQuery;

if (isFilterNone(query, result)) {
return new SearchResultList<>(new ArrayList<>());
if (checkNoneFilterAfterAutz(query, result)) {
return SearchResultList.empty();
}

enterModelMethod(); // outside try-catch because if this ends with an exception, cache is not entered yet
Expand Down Expand Up @@ -824,15 +831,17 @@ public <T extends Containerable> SearchResultMetadata searchContainersIterative(
var parsedOptions = preProcessOptionsSecurity(rawOptionsReadWrite, task, result);
var options = parsedOptions.getCollection();

if (checkNoneFilterBeforeAutz(query)) {
return null;
}

var ctx = new ContainerSearchLikeOpContext<>(type, origQuery, parsedOptions, task, result);

GetOperationOptions rootOptions = parsedOptions.getRootOptions();

ObjectQuery processedQuery = ctx.securityRestrictedQuery;


if (isFilterNone(processedQuery, result)) {
LOGGER.trace("Skipping search because filter is NONE");
if (checkNoneFilterAfterAutz(processedQuery, result)) {
return null;
}

Expand Down Expand Up @@ -914,10 +923,14 @@ public <T extends Containerable> Integer countContainers(Class<T> type, ObjectQu

try {
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);

if (checkNoneFilterBeforeAutz(query)) {
return 0;
}
var ctx = new ContainerSearchLikeOpContext<>(type, query, parsedOptions, task, result);
query = ctx.securityRestrictedQuery;

if (isFilterNone(query, result)) {
if (checkNoneFilterAfterAutz(query, result)) {
return 0;
}

Expand Down Expand Up @@ -947,10 +960,18 @@ public <T extends Containerable> Integer countContainers(Class<T> type, ObjectQu

// See MID-6323 in Jira

private boolean isFilterNone(ObjectQuery query, OperationResult result) {
if (query != null && query.getFilter() != null && query.getFilter() instanceof NoneFilter) {
LOGGER.trace("Security denied the search");
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Denied");
private boolean checkNoneFilterBeforeAutz(ObjectQuery query) {
if (ObjectQueryUtil.isNoneQuery(query)) {
LOGGER.trace("Skipping the search/count operation, as the NONE filter was requested");
return true;
}
return false;
}

private boolean checkNoneFilterAfterAutz(ObjectQuery query, OperationResult result) {
if (ObjectQueryUtil.isNoneQuery(query)) {
LOGGER.trace("Security denied the search/coount operation");
result.setNotApplicable("Denied"); // TODO really do we want "not applicable" here?
return true;
}
return false;
Expand Down Expand Up @@ -1000,10 +1021,13 @@ public <T extends ObjectType> SearchResultMetadata searchObjectsIterative(Class<
ObjectManager searchProvider = getObjectManager(type, options);
result.addArbitraryObjectAsParam("searchProvider", searchProvider);

if (checkNoneFilterBeforeAutz(query)) {
return null;
}

// see MID-6115
ObjectQuery processedQuery = preProcessQuerySecurity(type, query, rootOptions, task, result);
if (isFilterNone(processedQuery, result)) {
LOGGER.trace("Skipping search because filter is NONE");
if (checkNoneFilterAfterAutz(processedQuery, result)) {
return null;
}

Expand Down Expand Up @@ -1109,26 +1133,22 @@ public <T extends ObjectType> Integer countObjects(Class<T> type, ObjectQuery or
var rootOptions = parsedOptions.getRootOptions();
var options = parsedOptions.getCollection();

if (checkNoneFilterBeforeAutz(query)) {
return 0;
}

ObjectQuery processedQuery = preProcessQuerySecurity(type, query, rootOptions, task, result);
if (isFilterNone(processedQuery, result)) {
LOGGER.trace("Skipping count because filter is NONE");
if (checkNoneFilterAfterAutz(processedQuery, result)) {
return 0;
}

ObjectManager objectManager = getObjectManager(type, options);
switch (objectManager) {
case PROVISIONING:
count = provisioning.countObjects(type, processedQuery, options, task, parentResult);
break;
case REPOSITORY:
count = cacheRepositoryService.countObjects(type, processedQuery, options, parentResult);
break;
case TASK_MANAGER:
count = taskManager.countObjects(type, processedQuery, parentResult);
break;
default:
throw new AssertionError("Unexpected objectManager: " + objectManager);
}
count = switch (objectManager) {
case PROVISIONING -> provisioning.countObjects(type, processedQuery, options, task, parentResult);
case REPOSITORY -> cacheRepositoryService.countObjects(type, processedQuery, options, parentResult);
case TASK_MANAGER -> taskManager.countObjects(type, processedQuery, parentResult);
default -> throw new AssertionError("Unexpected objectManager: " + objectManager);
};
} catch (Throwable t) {
ModelImplUtils.recordException(result, t);
throw t;
Expand Down Expand Up @@ -1208,10 +1228,14 @@ public SearchResultList<ObjectReferenceType> searchReferences(
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
var options = parsedOptions.getCollection();

if (checkNoneFilterBeforeAutz(query)) {
return SearchResultList.empty();
}

query = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet!

if (isFilterNone(query, result)) {
return new SearchResultList<>(new ArrayList<>());
if (checkNoneFilterAfterAutz(query, result)) {
return SearchResultList.empty();
}

SearchResultList<ObjectReferenceType> list;
Expand Down Expand Up @@ -1245,33 +1269,37 @@ public Integer countReferences(ObjectQuery query,
Objects.requireNonNull(query, "Query must be provided for reference search");
Validate.notNull(parentResult, "Result type must not be null.");

OperationResult operationResult = parentResult.createSubresult(COUNT_REFERENCES)
OperationResult result = parentResult.createSubresult(COUNT_REFERENCES)
.addParam(OperationResult.PARAM_QUERY, query);

try {
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, operationResult);
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
var options = parsedOptions.getCollection();

query = preProcessReferenceQuerySecurity(query, task, operationResult); // TODO not implemented yet!
if (checkNoneFilterBeforeAutz(query)) {
return 0;
}

query = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet!

if (isFilterNone(query, operationResult)) {
if (checkNoneFilterAfterAutz(query, result)) {
return 0;
}

enterModelMethod(); // outside try-catch because if this ends with an exception, cache is not entered yet
try {
logQuery(query);

return cacheRepositoryService.countReferences(query, options, operationResult);
return cacheRepositoryService.countReferences(query, options, result);
} catch (RuntimeException e) {
recordSearchException(e, ObjectManager.REPOSITORY, operationResult);
recordSearchException(e, ObjectManager.REPOSITORY, result);
throw e;
} finally {
exitModelMethod();
}
} finally {
operationResult.close();
operationResult.cleanup();
result.close();
result.cleanup();
}
}

Expand All @@ -1295,16 +1323,18 @@ public SearchResultMetadata searchReferencesIterative(
OP_LOGGER.trace("MODEL OP enter searchReferencesIterative({}, {})", query, rawOptions);

ModelImplUtils.validatePaging(query.getPaging());
OperationResult operationResult = parentResult.createSubresult(SEARCH_REFERENCES)
OperationResult result = parentResult.createSubresult(SEARCH_REFERENCES)
.addParam(OperationResult.PARAM_QUERY, query);

try {
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, operationResult);
var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
var options = parsedOptions.getCollection();

ObjectQuery processedQuery = preProcessReferenceQuerySecurity(query, task, operationResult); // TODO not implemented yet!
if (isFilterNone(processedQuery, operationResult)) {
LOGGER.trace("Skipping search because filter is NONE");
if (checkNoneFilterBeforeAutz(query)) {
return null;
}
ObjectQuery processedQuery = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet!
if (checkNoneFilterAfterAutz(processedQuery, result)) {
return null;
}

Expand All @@ -1329,18 +1359,18 @@ public SearchResultMetadata searchReferencesIterative(
logQuery(processedQuery);

metadata = cacheRepositoryService.searchReferencesIterative(
processedQuery, internalHandler, options, operationResult);
processedQuery, internalHandler, options, result);
} catch (SchemaException | RuntimeException e) {
recordSearchException(e, ObjectManager.REPOSITORY, operationResult);
recordSearchException(e, ObjectManager.REPOSITORY, result);
throw e;
} finally {
exitModelMethodNoRepoCache();
}

return metadata;
} finally {
operationResult.close();
operationResult.cleanup();
result.close();
result.cleanup();
}
}
// endregion
Expand Down
2 changes: 1 addition & 1 deletion ...a/com/evolveum/midpoint/provisioning/impl/operations/ProvisioningSearchLikeOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public ProvisioningSearchLikeOperation(
ConfigurationException, ObjectNotFoundException {

if (filter instanceof NoneFilter) {
SearchResultList<PrismObject<T>> objListType = new SearchResultList<>(new ArrayList<>());
SearchResultList<PrismObject<T>> objListType = SearchResultList.empty();
objListType.setMetadata(createNoneFilterMetadata());
return objListType;
}
Expand Down

0 comments on commit 0d70b1e

Please sign in to comment.