Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Resource Sharing] Keep track of resource_type on resource sharing document ([#5772](https://github.com/opensearch-project/security/pull/5772))
- Add support for X509 v3 extensions (SAN) for authentication ([#5701](https://github.com/opensearch-project/security/pull/5701))
- [Resource Sharing] Requires default_owner for resource/migrate API ([#5789](https://github.com/opensearch-project/security/pull/5789))
- Optimize getFieldFilter to only return a predicate when index has FLS restrictions for user ([#5777](https://github.com/opensearch-project/security/pull/5777))


### Bug Fixes
- Create a WildcardMatcher.NONE when creating a WildcardMatcher with an empty string ([#5694](https://github.com/opensearch-project/security/pull/5694))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2341,13 +2341,24 @@ public Collection<Class<? extends LifecycleComponent>> getGuiceServiceClasses()
public Function<String, Predicate<String>> getFieldFilter() {
return index -> {
if (threadPool == null || dlsFlsValve == null) {
return field -> true;
return NOOP_FIELD_PREDICATE;
}

PrivilegesEvaluationContext ctx = this.dlsFlsBaseContext != null
? this.dlsFlsBaseContext.getPrivilegesEvaluationContext()
: null;

boolean indexHasRestrictions = false;
try {
indexHasRestrictions = dlsFlsValve.indexHasFlsRestrictions(index, ctx);
} catch (PrivilegesEvaluationException e) {
log.error("Error while evaluating FLS restrictions for {}", index, e);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the exception be re-thrown?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this block into the try-catch

if (!indexHasRestrictions) {
    return NOOP_FIELD_PREDICATE;
}

In the event of an error, it will use the existing field-level check which I think is sensible.

}

if (!indexHasRestrictions) {
return NOOP_FIELD_PREDICATE;
}

return field -> {
try {
return dlsFlsValve.isFieldAllowed(index, field, ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public interface DlsFlsRequestValve {

boolean isFieldAllowed(String index, String field, PrivilegesEvaluationContext ctx) throws PrivilegesEvaluationException;

boolean indexHasFlsRestrictions(String index, PrivilegesEvaluationContext ctx) throws PrivilegesEvaluationException;

public static class NoopDlsFlsRequestValve implements DlsFlsRequestValve {

@Override
Expand Down Expand Up @@ -87,6 +89,11 @@ public boolean hasFieldMasking(String index) {
public boolean isFieldAllowed(String index, String field, PrivilegesEvaluationContext ctx) {
return true;
}

@Override
public boolean indexHasFlsRestrictions(String index, PrivilegesEvaluationContext ctx) {
return false;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,15 @@ public boolean isFieldAllowed(String index, String field, PrivilegesEvaluationCo
return config.getFieldPrivileges().getRestriction(ctx, index).isAllowedRecursive(field);
}

@Override
public boolean indexHasFlsRestrictions(String index, PrivilegesEvaluationContext ctx) throws PrivilegesEvaluationException {
if (ctx == null) {
return false;
}
DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get();
return !config.getFieldPrivileges().getRestriction(ctx, index).isUnrestricted();
}

private static InternalAggregation aggregateBuckets(InternalAggregation aggregation) {
if (aggregation instanceof StringTerms) {
StringTerms stringTerms = (StringTerms) aggregation;
Expand Down
Loading