Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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 @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Refactoring

### Maintenance

- Update delete_backport_branch workflow to include release-chores branches ([#5548](https://github.com/opensearch-project/security/pull/5548))
- Bump `1password/load-secrets-action` from 2 to 3 ([#5573](https://github.com/opensearch-project/security/pull/5573))
- Bump `jjwt_version` from 0.12.6 to 0.13.0 ([#5568](https://github.com/opensearch-project/security/pull/5568), [#5581](https://github.com/opensearch-project/security/pull/5581))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ private void ingestData(String index) throws IOException {
bulkRequestBody.append(Song.randomSong().asJson() + "\n");
}
List<Response> responses = RestHelper.requestAgainstAllNodes(
testUserRestClient,
adminClient(),
"POST",
"_bulk?refresh=wait_for",
new StringEntity(bulkRequestBody.toString(), APPLICATION_NDJSON)
Expand Down Expand Up @@ -413,30 +413,31 @@ private boolean resourceExists(String url) throws IOException {
*/
private void createTestRoleIfNotExists(String role) throws IOException {
String url = "_plugins/_security/api/roles/" + role;
String roleSettings = "{\n"
+ " \"cluster_permissions\": [\n"
+ " \"unlimited\"\n"
+ " ],\n"
+ " \"index_permissions\": [\n"
+ " {\n"
+ " \"index_patterns\": [\n"
+ " \"test_index*\"\n"
+ " ],\n"
+ " \"dls\": \"{ \\\"bool\\\": { \\\"must\\\": { \\\"match\\\": { \\\"genre\\\": \\\"rock\\\" } } } }\",\n"
+ " \"fls\": [\n"
+ " \"~lyrics\"\n"
+ " ],\n"
+ " \"masked_fields\": [\n"
+ " \"artist\"\n"
+ " ],\n"
+ " \"allowed_actions\": [\n"
+ " \"read\",\n"
+ " \"write\"\n"
+ " ]\n"
+ " }\n"
+ " ],\n"
+ " \"tenant_permissions\": []\n"
+ "}\n";
String roleSettings = """
{
"cluster_permissions": [
"unlimited"
],
"index_permissions": [
{
"index_patterns": [
"test_index*"
],
"dls": "{ \"bool\": { \"must\": { \"match\": { \"genre\": \"rock\" } } } }",
"fls": [
"~lyrics"
],
"masked_fields": [
"artist"
],
"allowed_actions": [
"read",
]
}
],
"tenant_permissions": []
}
""";
Response response = RestHelper.makeRequest(adminClient(), "PUT", url, RestHelper.toHttpEntity(roleSettings));

assertThat(response.getStatusLine().getStatusCode(), anyOf(equalTo(200), equalTo(201)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,12 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<

if (request instanceof BulkShardRequest) {
for (BulkItemRequest inner : ((BulkShardRequest) request).items()) {
if (inner.request() instanceof UpdateRequest) {
listener.onFailure(
new OpenSearchSecurityException("Update is not supported when FLS or DLS or Fieldmasking is activated")
);
return false;
}
listener.onFailure(
new OpenSearchSecurityException(
inner.request().getClass().getSimpleName() + " is not supported when FLS or DLS or Fieldmasking is activated"
)
);
return false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
if (pres.isAllowed()) {
auditLog.logGrantedPrivileges(action, request, task);
auditLog.logIndexEvent(action, request, task);
if (!dlsFlsValve.invoke(context, listener)) {
if (!pres.shouldSkipDlsValve() && !dlsFlsValve.invoke(context, listener)) {
return;
}
final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder();
Expand All @@ -465,6 +465,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
createIndexRequest.index(),
alias2Name(createIndexRequest.aliases())
);
threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true");
createIndexRequestBuilder.execute(ActionListener.wrap(createIndexResponse -> {
if (createIndexResponse.isAcknowledged()) {
log.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
if (replaceResult.accessDenied) {
auditLog.logMissingPrivileges(action0, request, task);
} else {
presponse.shouldSkipDlsValve = true;
Copy link

Choose a reason for hiding this comment

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

hmm, what happens without this?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is needed in the first instance that a private tenant is created. OpenSearch Dashboards will try to save a saved object to the private tenant index which is an index request. If the private tenant index does not yet exist it will follow-up by trying to auto create the index. These actions need to still be permitted.

presponse.allowed = true;
presponse.createIndexRequestBuilder = replaceResult.createIndexRequestBuilder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class PrivilegesEvaluatorResponse {
private CheckTable<String, String> indexToActionCheckTable;
private String privilegeMatrix;
private String reason;
boolean shouldSkipDlsValve = false;

/**
* Contains issues that were encountered during privilege evaluation. Can be used for logging.
Expand All @@ -61,6 +62,13 @@ public boolean isAllowed() {
return allowed;
}

/**
* Returns true if the request is only for dashboards indices
*/
public boolean shouldSkipDlsValve() {
return shouldSkipDlsValve;
}

/**
* Returns true if the request can be allowed if the referenced indices are reduced (aka "do not fail on forbidden").
* See getAvailableIndices() for the indices for which we have privileges.
Expand Down
Loading