Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ 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))
- Add --timeout (-to) as an option to securityadmin.sh ([#5787](https://github.com/opensearch-project/security/pull/5787))

### Bug Fixes
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);

if (!indexHasRestrictions) {
return NOOP_FIELD_PREDICATE;
}
} 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.

}

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