diff --git a/MAINTAINERS.md b/MAINTAINERS.md index db68683708..69dd039815 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -1,40 +1,57 @@ -- [OpenSearch Security Maintainers](#opensearch-security-maintainers) - - [Maintainers](#maintainers) - - [Practices](#practices) - - [Updating Practices](#updating-practices) -- [Practices](#practices-1) +- [Overview](#overview) +- [Current Maintainers](#current-maintainers) +- [Practices](#practices) + - [Updating Practices](#updating-practices) - [Reverting Commits](#reverting-commits) - - [Performing Revert](#performing-revert) + - [Performing Revert](#performing-revert) + - [Squashing a Pull Request](#squashing-a-pull-request) -# OpenSearch Security Maintainers +## Overview + +This document contains a list of maintainers in this repo. See [opensearch-project/.github/RESPONSIBILITIES.md](https://github.com/opensearch-project/.github/blob/main/RESPONSIBILITIES.md#maintainer-responsibilities) that explains what the role of maintainer means, what maintainers do in this and other repos, and how they should be doing it. If you're interested in contributing, and becoming a maintainer, see [CONTRIBUTING](CONTRIBUTING.md). + +## Current Maintainers -## Maintainers | Maintainer | GitHub ID | Affiliation | -| ---------------- | ----------------------------------------------------- | ----------- | -| Chang Liu | [cliu123](https://github.com/cliu123) | Amazon | +|------------------|-------------------------------------------------------|-------------| | Darshit Chanpura | [DarshitChanpura](https://github.com/DarshitChanpura) | Amazon | -| Dave Lago | [davidlago](https://github.com/davidlago) | Amazon | | Peter Nied | [peternied](https://github.com/peternied) | Amazon | | Craig Perkins | [cwperks](https://github.com/cwperks) | Amazon | | Ryan Liang | [RyanL1997](https://github.com/RyanL1997) | Amazon | -| Stephen Crawford | [scrawfor99](https://github.com/scrawfor99) | Amazon | +| Stephen Crawford | [scrawfor99](https://github.com/stephen-crawford) | Amazon | | Andriy Redko | [reta](https://github.com/reta) | Aiven | | Andrey Pleskach | [willyborankin](https://github.com/willyborankin) | Aiven | -| Nils Bandener | [nibix](https://github.com/nibix) | Eliatra | + +## Emeritus + +| Maintainer | GitHub ID | Affiliation | +|------------|-------------------------------------------|-------------| +| Dave Lago | [davidlago](https://github.com/davidlago) | Contributor | +| Chang Liu | [cliu123](https://github.com/cliu123) | Amazon | ## Practices ### Updating Practices -To ensure common practices as maintainers, all practices are expected to be documented here or enforced through github actions. There should be no expectations beyond what is documented in the repo [CONTRIBUTING.md](./CONTRIBUTING.md) and OpenSearch-Project [CONTRIBUTING.md](https://github.com/opensearch-project/.github/blob/main/CONTRIBUTING.md). To modify an existing processes or create a new one, make a pull request on this MAINTAINERS.md for review and merge it after all maintainers approve of it. -# Practices +To ensure common practices as maintainers, all practices are expected to be documented here or enforced through github actions. There should be no expectations beyond what is documented in the repo [CONTRIBUTING.md](./CONTRIBUTING.md) and OpenSearch-Project [CONTRIBUTING.md](https://github.com/opensearch-project/.github/blob/main/CONTRIBUTING.md). To modify an existing processes or create a new one, make a pull request on this MAINTAINERS.md for review and merge it after all maintainers approve of it. + +### Reverting Commits -## Reverting Commits There will be changes that destabilize or block contributions. The impact of these changes will be localized on the repository or even the entire OpenSearch project. We should bias towards keeping contributions unblocked by immediately reverting impacting changes, these reverts will be done by a maintainer. After the change has been reverted, an issue will be openned to re-merge the change and callout the elements of the contribution that need extra examination such as additional tests or even pull request workflows. Exceptional, instead of immediately reverting, if a contributor knows how and will resolve the issue in an hour or less we should fix-forward to reduce overhead. ### Performing Revert + Go to the pull request of the change that was an issue, there is a `Revert` button at the bottom. If there are no conflicts to resolve, this can be done immediately bypassing standard approval. Reverts can also be done via the command line using `git revert ` and creating a new pull request. If done in this way they should have references to the pull request that was reverted. + +### Squashing a Pull Request + +When a PR is going to be merged, our repositories are set to automatically squash the commits into a single commit. This process needs human intervention to produce high quality commit messages, with the following steps to be followed as much as possible: + +- The commit subject is clean and conveys what is being merged +- The commit body should include the details (if any) about the commit, typically inline with the PR description +- The commit body should include the 'Signed-Off-By:*' for all committers involved in the change. +- There need to be a matching 'Signed-Off-By:' line for the `This commit will be authored by *` email address otherwise backport DCO checks will fail. diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java index 599ffe9ad2..add98ca572 100644 --- a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java @@ -80,4 +80,29 @@ public void adminShouldNotBeAbleToDeleteSecurityIndex() { assertThat(response4.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); } } + + @Test + public void regularUserShouldGetNoResultsWhenSearchingSystemIndex() { + // Create system index and index a dummy document as the super admin user, data returned to super admin + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + HttpResponse response1 = client.put(".system-index1"); + + assertThat(response1.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + String doc = "{\"field\":\"value\"}"; + HttpResponse adminPostResponse = client.postJson(".system-index1/_doc/1?refresh=true", doc); + assertThat(adminPostResponse.getStatusCode(), equalTo(RestStatus.CREATED.getStatus())); + HttpResponse response2 = client.get(".system-index1/_search"); + + assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response2.getBody(), response2.getBody().contains("\"hits\":{\"total\":{\"value\":1,\"relation\":\"eq\"}")); + } + + // Regular users should not be able to read it + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + // regular user cannot read system index + HttpResponse response1 = client.get(".system-index1/_search"); + + assertThat(response1.getBody(), response1.getBody().contains("\"hits\":{\"total\":{\"value\":0,\"relation\":\"eq\"}")); + } + } } diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java index ae3d1c9128..20e0b25b5c 100644 --- a/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java +++ b/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java @@ -88,6 +88,7 @@ public class HTTPSamlAuthenticator implements HTTPAuthenticator, Destroyable { private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); private static boolean openSamlInitialized = false; + public static final String SAML_TYPE = "saml"; private String subjectKey; private String rolesKey; @@ -175,7 +176,7 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T @Override public String getType() { - return "saml"; + return SAML_TYPE; } @Override diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index d633d307e9..f06b5aa56d 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -75,6 +75,7 @@ import static org.apache.http.HttpStatus.SC_FORBIDDEN; import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE; import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; +import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.SAML_TYPE; public class BackendRegistry { @@ -303,7 +304,10 @@ public boolean authenticate(final SecurityRequestChannel request) { if (authDomain.isChallenge()) { final Optional restResponse = httpAuthenticator.reRequestAuthentication(request, null); if (restResponse.isPresent()) { - auditLog.logFailedLogin("", false, null, request); + // saml will always hit this to re-request authentication + if (!authDomain.getHttpAuthenticator().getType().equals(SAML_TYPE)) { + auditLog.logFailedLogin("", false, null, request); + } if (isTraceEnabled) { log.trace("No 'Authorization' header, send 401 and 'WWW-Authenticate Basic'"); } diff --git a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java index f66ff7a2c0..e889368315 100644 --- a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java +++ b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java @@ -41,7 +41,7 @@ import org.opensearch.security.support.HeaderHelper; import org.opensearch.security.support.SecurityUtils; -public class SecurityFlsDlsIndexSearcherWrapper extends SecurityIndexSearcherWrapper { +public class SecurityFlsDlsIndexSearcherWrapper extends SystemIndexSearcherWrapper { public final Logger log = LogManager.getLogger(this.getClass()); diff --git a/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java similarity index 93% rename from src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java rename to src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java index 7a40e5dbd0..8e89b60712 100644 --- a/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java +++ b/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java @@ -39,6 +39,7 @@ import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.core.index.Index; import org.opensearch.index.IndexService; +import org.opensearch.indices.SystemIndexRegistry; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.securityconf.ConfigModel; import org.opensearch.security.securityconf.SecurityRoles; @@ -49,7 +50,7 @@ import org.greenrobot.eventbus.Subscribe; -public class SecurityIndexSearcherWrapper implements CheckedFunction { +public class SystemIndexSearcherWrapper implements CheckedFunction { protected final Logger log = LogManager.getLogger(this.getClass()); protected final ThreadContext threadContext; @@ -68,7 +69,7 @@ public class SecurityIndexSearcherWrapper implements CheckedFunction mappedRoles = evaluator.mapRoles(user, caller);