Skip to content

Commit

Permalink
Adds saml auth header to differentiate saml requests and prevents aut…
Browse files Browse the repository at this point in the history
…o login as anonymous user when basic authentication fails (#4152)

Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura authored Apr 9, 2024
1 parent ba74d14 commit d87ab3f
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/
package org.opensearch.security.http;

import java.util.Map;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain;
import org.opensearch.test.framework.TestSecurityConfig.User;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;

import static org.apache.http.HttpStatus.SC_OK;
import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class BasicWithAnonymousAuthTests {
static final User TEST_USER = new User("test_user").password("s3cret");

public static final String CUSTOM_ATTRIBUTE_NAME = "superhero";
static final User SUPER_USER = new User("super-user").password("super-password").attr(CUSTOM_ATTRIBUTE_NAME, "true");
public static final String NOT_EXISTING_USER = "not-existing-user";
public static final String INVALID_PASSWORD = "secret-password";

public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal");

@ClassRule
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
.anonymousAuth(true)
.authc(AUTHC_DOMAIN)
.users(TEST_USER, SUPER_USER)
.build();

/** No automatic login post anonymous auth request **/
@Test
public void testShouldRespondWith401WhenUserDoesNotExist() {
try (TestRestClient client = cluster.getRestClient(NOT_EXISTING_USER, INVALID_PASSWORD)) {
HttpResponse response = client.getAuthInfo();

assertThat(response, is(notNullValue()));
response.assertStatusCode(SC_UNAUTHORIZED);
}
}

@Test
public void testShouldRespondWith401WhenUserNameIsIncorrect() {
try (TestRestClient client = cluster.getRestClient(NOT_EXISTING_USER, TEST_USER.getPassword())) {
HttpResponse response = client.getAuthInfo();

assertThat(response, is(notNullValue()));
response.assertStatusCode(SC_UNAUTHORIZED);
}
}

@Test
public void testShouldRespondWith401WhenPasswordIsIncorrect() {
try (TestRestClient client = cluster.getRestClient(TEST_USER.getName(), INVALID_PASSWORD)) {
HttpResponse response = client.getAuthInfo();

assertThat(response, is(notNullValue()));
response.assertStatusCode(SC_UNAUTHORIZED);
}
}

/** Test `?auth_type=""` param to authinfo request **/
@Test
public void testShouldAutomaticallyLoginAsAnonymousIfNoCredentialsArePassed() {
try (TestRestClient client = cluster.getRestClient()) {

HttpResponse response = client.getAuthInfo();

assertThat(response, is(notNullValue()));
response.assertStatusCode(SC_OK);

HttpResponse response2 = client.getAuthInfo(Map.of("auth_type", "anonymous"));

assertThat(response2, is(notNullValue()));
response2.assertStatusCode(SC_OK);
}
}

@Test
public void testShouldNotAutomaticallyLoginAsAnonymousIfRequestIsNonAnonymousLogin() {
try (TestRestClient client = cluster.getRestClient()) {

HttpResponse response = client.getAuthInfo(Map.of("auth_type", "saml"));

assertThat(response, is(notNullValue()));
response.assertStatusCode(SC_UNAUTHORIZED);

// should contain a redirect link
assertThat(response.containHeader("WWW-Authenticate"), is(true));
}
}
}
47 changes: 32 additions & 15 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
Expand All @@ -44,6 +45,7 @@
import com.google.common.cache.RemovalListener;
import com.google.common.cache.RemovalNotification;
import com.google.common.collect.Multimap;
import org.apache.http.HttpHeaders;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down Expand Up @@ -190,7 +192,7 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
* @param request
* @return The authenticated user, null means another roundtrip
* @throws OpenSearchSecurityException
*/
*/
public boolean authenticate(final SecurityRequestChannel request) {
final boolean isDebugEnabled = log.isDebugEnabled();
final boolean isBlockedBasedOnAddress = request.getRemoteAddress()
Expand Down Expand Up @@ -286,7 +288,7 @@ public boolean authenticate(final SecurityRequestChannel request) {

if (ac == null) {
// no credentials found in request
if (anonymousAuthEnabled) {
if (anonymousAuthEnabled && isRequestForAnonymousLogin(request.params(), request.getHeaders())) {
continue;
}

Expand Down Expand Up @@ -386,19 +388,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size());
}

if (authCredentials == null && anonymousAuthEnabled) {
final String tenant = resolveTenantFrom(request);
User anonymousUser = new User(User.ANONYMOUS.getName(), new HashSet<String>(User.ANONYMOUS.getRoles()), null);
anonymousUser.setRequestedTenant(tenant);

threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, anonymousUser);
auditLog.logSucceededLogin(anonymousUser.getName(), false, null, request);
if (isDebugEnabled) {
log.debug("Anonymous User is authenticated");
}
return true;
}

Optional<SecurityResponse> challengeResponse = Optional.empty();

if (firstChallengingHttpAuthenticator != null) {
Expand All @@ -415,6 +404,19 @@ public boolean authenticate(final SecurityRequestChannel request) {
}
}

if (authCredentials == null && anonymousAuthEnabled && isRequestForAnonymousLogin(request.params(), request.getHeaders())) {
final String tenant = resolveTenantFrom(request);
User anonymousUser = new User(User.ANONYMOUS.getName(), new HashSet<String>(User.ANONYMOUS.getRoles()), null);
anonymousUser.setRequestedTenant(tenant);

threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, anonymousUser);
auditLog.logSucceededLogin(anonymousUser.getName(), false, null, request);
if (isDebugEnabled) {
log.debug("Anonymous User is authenticated");
}
return true;
}

log.warn(
"Authentication finally failed for {} from {}",
authCredentials == null ? null : authCredentials.getUsername(),
Expand All @@ -432,6 +434,21 @@ public boolean authenticate(final SecurityRequestChannel request) {
return authenticated;
}

/**
* Checks if incoming auth request is from an anonymous user
* Defaults all requests to yes, to allow anonymous authentication to succeed
* @param params the query parameters passed in this request
* @return false only if an explicit `auth_type` param is supplied, and its value is not anonymous, OR
* if request contains no authorization headers
* otherwise returns true
*/
private boolean isRequestForAnonymousLogin(Map<String, String> params, Map<String, List<String>> headers) {
if (params.containsKey("auth_type")) {
return params.get("auth_type").equals("anonymous");
}
return !headers.containsKey(HttpHeaders.AUTHORIZATION);
}

private String resolveTenantFrom(final SecurityRequest request) {
return Optional.ofNullable(request.header("securitytenant")).orElse(request.header("security_tenant"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ public List<Route> routes() {
@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
final boolean verbose = request.paramAsBoolean("verbose", false);
// need to consume `auth_type` param, without which a 500 is thrown on front-end
final String authType = request.param("auth_type", "");

return new RestChannelConsumer() {

@Override
Expand Down

0 comments on commit d87ab3f

Please sign in to comment.