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 @@ -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