diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 376ce0b2b8..b3732685cb 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -167,7 +167,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() { try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { TestRestClient.HttpResponse response = client.postJson(CREATE_OBO_TOKEN_PATH, OBO_DESCRIPTION); - response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED); + response.assertStatusCode(HttpStatus.SC_FORBIDDEN); } } @@ -178,7 +178,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() { try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { TestRestClient.HttpResponse response = client.putJson("_plugins/_security/api/account", CURRENT_AND_NEW_PASSWORDS); - response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED); + response.assertStatusCode(HttpStatus.SC_FORBIDDEN); } } diff --git a/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java b/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java index 30735d46c0..7666e32dbb 100644 --- a/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java +++ b/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java @@ -29,6 +29,8 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.security.identity.SecurityTokenManager; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.user.User; import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.rest.RestRequest.Method.POST; @@ -84,6 +86,14 @@ public void accept(final RestChannel channel) throws Exception { final XContentBuilder builder = channel.newBuilder(); BytesRestResponse response; try { + final User user = (User) client.threadPool().getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); + if (user != null && isTokenAuthenticatedUser(user)) { + channel.sendResponse( + new BytesRestResponse(RestStatus.FORBIDDEN, "Token-authenticated users cannot create new tokens") + ); + return; + } + if (!securityTokenManager.issueOnBehalfOfTokenAllowed()) { channel.sendResponse( new BytesRestResponse( @@ -163,4 +173,9 @@ private long parseAndValidateDurationSeconds(final Object durationObj) throws Il } throw new IllegalArgumentException("durationSeconds must be a number."); } + + private static boolean isTokenAuthenticatedUser(User user) { + String authBy = user.getAuthenticatedBy(); + return "onbehalfof_jwt".equals(authBy); + } } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 53baa1034e..af23799072 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -452,6 +452,8 @@ Authenticate users injected in the thread context by internal components (plugin Disallow superuser authentication through auth domain. Only client cert authentication is allowed for this user. */ + authenticatedUser.setAuthenticatedBy(authDomain.getHttpAuthenticator().getType()); + if (adminDns.isAdmin(authenticatedUser)) { log.error("Cannot authenticate user because admin user is not permitted to login via HTTP"); auditLog.logFailedLogin(authenticatedUser.getName(), true, null, request); diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index 260b4cbec1..f1e493a7e3 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -35,8 +35,6 @@ import com.nimbusds.jose.jwk.OctetSequenceKey; import com.nimbusds.jwt.SignedJWT; -import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; - public class JwtVendor { private static final Logger logger = LogManager.getLogger(JwtVendor.class); @@ -57,7 +55,7 @@ public JwtVendor(Settings settings) { * */ static Tuple createJwkFromSettings(final Settings settings) { final OctetSequenceKey key; - if (!isKeyNull(settings, "signing_key")) { + if (settings.get("signing_key") != null) { final String signingKey = settings.get("signing_key"); key = new OctetSequenceKey.Builder(Base64.getDecoder().decode(signingKey)).algorithm(JWSAlgorithm.HS512) .keyUse(KeyUse.SIGNATURE) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java index a74983733e..eb45a3bd80 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java @@ -42,6 +42,7 @@ import org.opensearch.threadpool.ThreadPool; import static org.opensearch.security.dlic.rest.api.Responses.badRequestMessage; +import static org.opensearch.security.dlic.rest.api.Responses.forbiddenMessage; import static org.opensearch.security.dlic.rest.api.Responses.ok; import static org.opensearch.security.dlic.rest.api.Responses.response; import static org.opensearch.security.dlic.rest.support.Utils.OPENDISTRO_API_DEPRECATION_MESSAGE; @@ -110,16 +111,21 @@ private void accountApiRequestHandlers(RequestHandler.RequestHandlersBuilder req userAccount(channel, user, remoteAddress, configuration); }).error((status, toXContent) -> response(channel, status, toXContent)) ) - .onChangeRequest( - Method.PUT, - request -> withUserAndRemoteAddress().map( - userAndRemoteAddress -> loadConfigurationWithRequestContent(userAndRemoteAddress.getLeft().getName(), request) - ) - .map(endpointValidator::entityExists) - .map(endpointValidator::onConfigChange) - .map(this::validCurrentPassword) - .map(this::updatePassword) - ); + .onChangeRequest(Method.PUT, request -> withUserAndRemoteAddress().map(userAndRemoteAddress -> { + final User user = userAndRemoteAddress.getLeft(); + if ("onbehalfof_jwt".equals(user.getAuthenticatedBy())) { + return ValidationResult.error( + RestStatus.FORBIDDEN, + forbiddenMessage("Token-authenticated users cannot change passwords") + ); + } + return ValidationResult.success(userAndRemoteAddress); + }) + .map(userAndRemoteAddress -> loadConfigurationWithRequestContent(userAndRemoteAddress.getLeft().getName(), request)) + .map(endpointValidator::entityExists) + .map(endpointValidator::onConfigChange) + .map(this::validCurrentPassword) + .map(this::updatePassword)); } private void userAccount( diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 7841c12208..b8a39f7d6b 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -17,7 +17,6 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.Set; -import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -25,7 +24,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.OpenSearchException; import org.opensearch.OpenSearchSecurityException; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; @@ -35,7 +33,6 @@ import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; import org.opensearch.security.filter.SecurityRequest; import org.opensearch.security.filter.SecurityResponse; -import org.opensearch.security.ssl.util.ExceptionUtils; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.KeyUtils; @@ -44,15 +41,9 @@ import io.jsonwebtoken.JwtParserBuilder; import io.jsonwebtoken.security.WeakKeyException; -import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; -import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; -import static org.opensearch.security.util.AuthTokenUtils.isAccessToRestrictedEndpoints; - public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private static final int MINIMUM_SIGNING_KEY_BIT_LENGTH = 512; - private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; - private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); protected final Logger log = LogManager.getLogger(this.getClass()); @@ -161,10 +152,6 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) { return null; } - if (!isRequestAllowed(request)) { - return null; - } - try { final Claims claims = jwtParser.parseClaimsJws(jwtToken).getBody(); @@ -249,17 +236,6 @@ private void logDebug(String message, Object... args) { } } - public Boolean isRequestAllowed(final SecurityRequest request) { - Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); - final String suffix = matcher.matches() ? matcher.group(2) : null; - if (isAccessToRestrictedEndpoints(request, suffix)) { - final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); - log.error(exception.toString()); - return false; - } - return true; - } - @Override public Optional reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) { return Optional.empty(); diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index fb24397a04..16e9f8ae86 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -66,8 +66,6 @@ import org.opensearch.security.securityconf.impl.v7.ConfigV7.AuthzDomain; import org.opensearch.security.support.ReflectionHelper; -import static org.opensearch.security.util.AuthTokenUtils.isKeyNull; - public class DynamicConfigModelV7 extends DynamicConfigModel { private final ConfigV7 config; @@ -370,7 +368,7 @@ private void buildAAA() { * order: -1 - prioritize the OBO authentication when it gets enabled */ Settings oboSettings = getDynamicOnBehalfOfSettings(); - if (!isKeyNull(oboSettings, "signing_key")) { + if (oboSettings.get("signing_key") != null) { final AuthDomain _ad = new AuthDomain( new NoOpAuthenticationBackend(Settings.EMPTY, null), new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), diff --git a/src/main/java/org/opensearch/security/user/User.java b/src/main/java/org/opensearch/security/user/User.java index aa2ac835f7..d2854be299 100644 --- a/src/main/java/org/opensearch/security/user/User.java +++ b/src/main/java/org/opensearch/security/user/User.java @@ -103,6 +103,12 @@ public static User fromSerializedBase64(String serializedBase64) { */ private volatile transient String serializedBase64; + /** + * The type of authenticator that authenticated this user (e.g., "basic", "obo", "apitoken"). + * Transient — not serialized, only used for in-process authorization decisions. + */ + private transient String authenticatedBy; + /** * Create a new authenticated user without roles and attributes * @@ -325,6 +331,14 @@ public String getPluginName() { return null; } + public String getAuthenticatedBy() { + return authenticatedBy; + } + + public void setAuthenticatedBy(String authenticatedBy) { + this.authenticatedBy = authenticatedBy; + } + /** * Returns a String containing serialized form of this User object. Never returns null. */ diff --git a/src/main/java/org/opensearch/security/util/AuthTokenUtils.java b/src/main/java/org/opensearch/security/util/AuthTokenUtils.java deleted file mode 100644 index 7bbe634c6c..0000000000 --- a/src/main/java/org/opensearch/security/util/AuthTokenUtils.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * 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. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.util; - -import org.opensearch.common.settings.Settings; -import org.opensearch.security.filter.SecurityRequest; - -import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.rest.RestRequest.Method.PUT; - -public class AuthTokenUtils { - private static final String ON_BEHALF_OF_SUFFIX = "api/obo/token"; - private static final String ON_BEHALF_OF_SUFFIX_DEPRECATED = "api/generateonbehalfoftoken"; - private static final String ACCOUNT_SUFFIX = "api/account"; - - public static Boolean isAccessToRestrictedEndpoints(final SecurityRequest request, final String suffix) { - if (suffix == null) { - return false; - } - switch (suffix) { - case ON_BEHALF_OF_SUFFIX: - case ON_BEHALF_OF_SUFFIX_DEPRECATED: - return request.method() == POST; - case ACCOUNT_SUFFIX: - return request.method() == PUT; - default: - return false; - } - } - - public static Boolean isKeyNull(Settings settings, String key) { - return settings.get(key) == null; - } -} diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java deleted file mode 100644 index 1024bfc954..0000000000 --- a/src/test/java/org/opensearch/security/authtoken/jwt/AuthTokenUtilsTest.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * 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. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.authtoken.jwt; - -import java.util.Collections; - -import org.junit.Test; - -import org.opensearch.common.settings.Settings; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.rest.RestRequest; -import org.opensearch.security.filter.SecurityRequestFactory; -import org.opensearch.security.util.AuthTokenUtils; -import org.opensearch.test.rest.FakeRestRequest; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -public class AuthTokenUtilsTest { - - @Test - public void testIsAccessToRestrictedEndpointsForOnBehalfOfToken() { - NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); - - FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/obo/token") - .withMethod(RestRequest.Method.POST) - .build(); - - assertTrue(AuthTokenUtils.isAccessToRestrictedEndpoints(SecurityRequestFactory.from(request), "api/obo/token")); - } - - @Test - public void testIsAccessToRestrictedEndpointsForAccount() { - NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); - - FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/account") - .withMethod(RestRequest.Method.PUT) - .build(); - - assertTrue(AuthTokenUtils.isAccessToRestrictedEndpoints(SecurityRequestFactory.from(request), "api/account")); - } - - @Test - public void testIsAccessToRestrictedEndpointsFalseCase() { - NamedXContentRegistry namedXContentRegistry = new NamedXContentRegistry(Collections.emptyList()); - - FakeRestRequest request = new FakeRestRequest.Builder(namedXContentRegistry).withPath("/api/someotherendpoint") - .withMethod(RestRequest.Method.GET) - .build(); - - assertFalse(AuthTokenUtils.isAccessToRestrictedEndpoints(SecurityRequestFactory.from(request), "api/someotherendpoint")); - } - - @Test - public void testIsKeyNullWithNullValue() { - Settings settings = Settings.builder().put("someKey", (String) null).build(); - assertTrue(AuthTokenUtils.isKeyNull(settings, "someKey")); - } - - @Test - public void testIsKeyNullWithNonNullValue() { - Settings settings = Settings.builder().put("someKey", "value").build(); - assertFalse(AuthTokenUtils.isKeyNull(settings, "someKey")); - } - - @Test - public void testIsKeyNullWithAbsentKey() { - Settings settings = Settings.builder().build(); - assertTrue(AuthTokenUtils.isKeyNull(settings, "absentKey")); - } -} diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index f183bbc3b1..e48168a8a4 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -37,7 +37,6 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.common.settings.Settings; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; -import org.opensearch.security.filter.SecurityRequest; import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; @@ -51,8 +50,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.rest.RestRequest.Method.PUT; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -627,27 +624,6 @@ public void testDifferentIssuer() throws Exception { assertNull(credentials); } - @Test - public void testRequestNotAllowed() { - OnBehalfOfAuthenticator oboAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName); - - // Test POST on generate on-behalf-of token endpoint - SecurityRequest mockedRequest1 = mock(SecurityRequest.class); - when(mockedRequest1.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); - when(mockedRequest1.path()).thenReturn(SECURITY_PREFIX + ON_BEHALF_OF_SUFFIX); - when(mockedRequest1.method()).thenReturn(POST); - assertFalse(oboAuth.isRequestAllowed(mockedRequest1)); - assertNull(oboAuth.extractCredentials(mockedRequest1, null)); - - // Test PUT on password changing endpoint - SecurityRequest mockedRequest2 = mock(SecurityRequest.class); - when(mockedRequest2.header(HttpHeaders.AUTHORIZATION)).thenReturn("Bearer someToken"); - when(mockedRequest2.path()).thenReturn(SECURITY_PREFIX + ACCOUNT_SUFFIX); - when(mockedRequest2.method()).thenReturn(PUT); - assertFalse(oboAuth.isRequestAllowed(mockedRequest2)); - assertNull(oboAuth.extractCredentials(mockedRequest2, null)); - } - /** extracts a default user credential from a request header */ private AuthCredentials extractCredentialsFromJwtHeader( final String signingKeyB64Encoded,