From bbe617effc15accab37aabb46d5e7e428ee57890 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Thu, 13 Jul 2023 14:50:53 -0700 Subject: [PATCH 01/14] OBO edge cases commit 0 Signed-off-by: Ryan Liang --- .../security/authtoken/jwt/JwtVendor.java | 3 ++ .../security/filter/SecurityRestFilter.java | 11 ++++++ .../security/ssl/util/ExceptionUtils.java | 4 ++ .../security/support/HTTPHelper.java | 39 +++++++++++++++++++ 4 files changed, 57 insertions(+) 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 3cec339647..1d38cda702 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -105,6 +105,7 @@ public String createJwt( List roles, List backendRoles ) throws Exception { + String tokenIdentifier = "OBO"; long timeMillis = timeProvider.getAsLong(); Instant now = Instant.ofEpochMilli(timeProvider.getAsLong()); @@ -112,6 +113,8 @@ public String createJwt( JwtClaims jwtClaims = new JwtClaims(); JwtToken jwt = new JwtToken(jwtClaims); + jwtClaims.setProperty("token_identifier", tokenIdentifier); + jwtClaims.setIssuer(issuer); jwtClaims.setIssuedAt(timeMillis); diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 54d98ffe13..622ca46f1d 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -259,6 +259,17 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha } } + if (HTTPHelper.containsOBOToken(request)) { + String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; + if (request.method() == Method.POST && OBO_ENDPOINT_PREFIX.equals(suffix)) { + final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); + log.error(exception.toString()); + auditLog.logBadHeaders(request); + channel.sendResponse(new BytesRestResponse(channel, RestStatus.FORBIDDEN, exception)); + return true; + } + } + return false; } diff --git a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java index 9d6d3dade8..5302287b60 100644 --- a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java +++ b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java @@ -64,6 +64,10 @@ public static OpenSearchException createBadHeaderException() { ); } + public static OpenSearchException invalidUsageOfOBOTokenException() { + return new OpenSearchException("On-Behalf-Token is not allowed to access this endopoint."); + } + public static OpenSearchException createTransportClientNoLongerSupportedException() { return new OpenSearchException("Transport client authentication no longer supported."); } diff --git a/src/main/java/org/opensearch/security/support/HTTPHelper.java b/src/main/java/org/opensearch/security/support/HTTPHelper.java index c3b191f770..4ef5235d53 100644 --- a/src/main/java/org/opensearch/security/support/HTTPHelper.java +++ b/src/main/java/org/opensearch/security/support/HTTPHelper.java @@ -31,6 +31,9 @@ import java.util.List; import java.util.Map; +import io.jsonwebtoken.Claims; +import io.jsonwebtoken.Jws; +import io.jsonwebtoken.Jwts; import org.apache.logging.log4j.Logger; import org.opensearch.rest.RestRequest; @@ -100,4 +103,40 @@ public static boolean containsBadHeader(final RestRequest request) { return false; } + + public static boolean containsOBOToken(final RestRequest request){ + final Map> headers; + + if (request != null && (headers = request.getHeaders()) != null) { + List authHeaders = headers.get("Authorization"); + if (authHeaders != null && !authHeaders.isEmpty()) { + // Iterate through the list of 'Authorization' headers, checking each for the 'Bearer' prefix. + for (String authHeader : authHeaders) { + if (authHeader != null && authHeader.startsWith("Bearer ")) { + // Header found, extract the token to verify it's an OBO token. + String token = authHeader.substring("Bearer ".length()); + if (isOBOToken(token)) { + return true; + } + } + } + } + } + + return false; + } + + private static boolean isOBOToken(String token) { + String tokenIdentifierClaimKey = "token_identifier"; + String tokenIdentifier = "OBO"; + + Jws claimsJws = Jwts.parserBuilder().build().parseClaimsJws(token); + Claims claims = claimsJws.getBody(); + + if (claims.containsKey(tokenIdentifierClaimKey) && tokenIdentifier.equals(claims.get(tokenIdentifierClaimKey))) { + return true; + } + return false; + } + } From 5a190ce4c229ae53dc7bf1e8f18da625f5b1d932 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Thu, 13 Jul 2023 16:04:28 -0700 Subject: [PATCH 02/14] Add a test case for using obo to create another obo + fix some lint and refactoring e2e testing Signed-off-by: Ryan Liang --- .../http/OnBehalfOfJwtAuthenticationTest.java | 73 +++++++++---------- .../framework/cluster/TestRestClient.java | 4 + .../security/support/HTTPHelper.java | 2 +- 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 9afebe457b..9d731328d5 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -19,6 +19,7 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.message.BasicHeader; +import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,6 +35,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; +import static org.junit.Assert; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; @@ -76,60 +78,51 @@ public class OnBehalfOfJwtAuthenticationTest { @Test public void shouldAuthenticateWithOBOTokenEndPoint() { - Header adminOboAuthHeader; - - try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) { - - client.assertCorrectCredentials(ADMIN_USER_NAME); - - TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON); - response.assertStatusCode(200); - - Map oboEndPointResponse = response.getBodyAs(Map.class); - assertThat(oboEndPointResponse, allOf(aMapWithSize(3), hasKey("user"), hasKey("onBehalfOfToken"), hasKey("duration"))); - - String encodedOboTokenStr = oboEndPointResponse.get("onBehalfOfToken").toString(); - - adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + encodedOboTokenStr); - } - - try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { - - TestRestClient.HttpResponse response = client.getAuthInfo(); - response.assertStatusCode(200); - - String username = response.getTextFromJsonBody(POINTER_USERNAME); - assertThat(username, equalTo(ADMIN_USER_NAME)); - } + String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); + Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); + authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, 200); } @Test public void shouldNotAuthenticateWithATemperedOBOToken() { - Header adminOboAuthHeader; + String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); + oboToken = oboToken.substring(0, oboToken.length() - 1); // tampering the token + Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); + authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, 401); + } - try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) { + @Test + public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() { + String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); + Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); - client.assertCorrectCredentials(ADMIN_USER_NAME); + try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { + TestRestClient.HttpResponse response = client.getOBOToken(adminOboAuthHeader); + response.assertStatusCode(403); + } + } + private String generateOboToken(String username, String password) { + try (TestRestClient client = cluster.getRestClient(username, password)) { + client.assertCorrectCredentials(username); TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON); response.assertStatusCode(200); - Map oboEndPointResponse = response.getBodyAs(Map.class); assertThat(oboEndPointResponse, allOf(aMapWithSize(3), hasKey("user"), hasKey("onBehalfOfToken"), hasKey("duration"))); - - String encodedOboTokenStr = oboEndPointResponse.get("onBehalfOfToken").toString(); - StringBuilder stringBuilder = new StringBuilder(encodedOboTokenStr); - stringBuilder.deleteCharAt(encodedOboTokenStr.length() - 1); - String temperedOboTokenStr = stringBuilder.toString(); - - adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + temperedOboTokenStr); + return oboEndPointResponse.get("onBehalfOfToken").toString(); } + } - try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { - + private void authenticateWithOboToken(Header authHeader, String expectedUsername, int expectedStatusCode) { + try (TestRestClient client = cluster.getRestClient(authHeader)) { TestRestClient.HttpResponse response = client.getAuthInfo(); - response.assertStatusCode(401); - response.getBody().contains("Unauthorized"); + response.assertStatusCode(expectedStatusCode); + if (expectedStatusCode == 200) { + String username = response.getTextFromJsonBody(POINTER_USERNAME); + assertThat(username, equalTo(expectedUsername)); + } else { + Assert.assertTrue(response.getBody().contains("Unauthorized")); + } } } } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index f446cac933..0d343c06bd 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -135,6 +135,10 @@ public HttpResponse getAuthInfo(Header... headers) { return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers); } + public HttpResponse getOBOToken(Header... headers) { + return executeRequest(new HttpPost(getHttpServerUri() + "/_plugin/_security/api/user/onbehalfof?pretty"), headers); + } + public void assertCorrectCredentials(String expectedUserName) { HttpResponse response = getAuthInfo(); assertThat(response, notNullValue()); diff --git a/src/main/java/org/opensearch/security/support/HTTPHelper.java b/src/main/java/org/opensearch/security/support/HTTPHelper.java index 4ef5235d53..a3c9202d02 100644 --- a/src/main/java/org/opensearch/security/support/HTTPHelper.java +++ b/src/main/java/org/opensearch/security/support/HTTPHelper.java @@ -104,7 +104,7 @@ public static boolean containsBadHeader(final RestRequest request) { return false; } - public static boolean containsOBOToken(final RestRequest request){ + public static boolean containsOBOToken(final RestRequest request) { final Map> headers; if (request != null && (headers = request.getHeaders()) != null) { From 171dd30b7e316c04199b04e89e86d3f7bf78732b Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Thu, 13 Jul 2023 16:15:21 -0700 Subject: [PATCH 03/14] Remove bad import of junit Assert Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfJwtAuthenticationTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 9d731328d5..0ef08fd2a8 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -35,7 +35,6 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; -import static org.junit.Assert; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; From 0dfc5ef7ba66508bae73da9362ede0d0860301b3 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Thu, 13 Jul 2023 16:19:16 -0700 Subject: [PATCH 04/14] Modify the exception msg Signed-off-by: Ryan Liang --- .../java/org/opensearch/security/ssl/util/ExceptionUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java index 5302287b60..972f7ea035 100644 --- a/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java +++ b/src/main/java/org/opensearch/security/ssl/util/ExceptionUtils.java @@ -65,7 +65,7 @@ public static OpenSearchException createBadHeaderException() { } public static OpenSearchException invalidUsageOfOBOTokenException() { - return new OpenSearchException("On-Behalf-Token is not allowed to access this endopoint."); + return new OpenSearchException("On-Behalf-Of Token is not allowed to be used for accessing this endopoint."); } public static OpenSearchException createTransportClientNoLongerSupportedException() { From 18dc94c70f3c765fc2e0ceeb8df71ff4a8913771 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 14 Jul 2023 11:03:13 -0700 Subject: [PATCH 05/14] Change the naming in token payload Signed-off-by: Ryan Liang --- .../java/org/opensearch/security/authtoken/jwt/JwtVendor.java | 4 ++-- src/main/java/org/opensearch/security/support/HTTPHelper.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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 1d38cda702..21d7caad57 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -105,7 +105,7 @@ public String createJwt( List roles, List backendRoles ) throws Exception { - String tokenIdentifier = "OBO"; + String tokenIdentifier = "obo"; long timeMillis = timeProvider.getAsLong(); Instant now = Instant.ofEpochMilli(timeProvider.getAsLong()); @@ -113,7 +113,7 @@ public String createJwt( JwtClaims jwtClaims = new JwtClaims(); JwtToken jwt = new JwtToken(jwtClaims); - jwtClaims.setProperty("token_identifier", tokenIdentifier); + jwtClaims.setProperty("typ", tokenIdentifier); jwtClaims.setIssuer(issuer); diff --git a/src/main/java/org/opensearch/security/support/HTTPHelper.java b/src/main/java/org/opensearch/security/support/HTTPHelper.java index a3c9202d02..b18bad83e6 100644 --- a/src/main/java/org/opensearch/security/support/HTTPHelper.java +++ b/src/main/java/org/opensearch/security/support/HTTPHelper.java @@ -127,8 +127,8 @@ public static boolean containsOBOToken(final RestRequest request) { } private static boolean isOBOToken(String token) { - String tokenIdentifierClaimKey = "token_identifier"; - String tokenIdentifier = "OBO"; + String tokenIdentifierClaimKey = "typ"; + String tokenIdentifier = "obo"; Jws claimsJws = Jwts.parserBuilder().build().parseClaimsJws(token); Claims claims = claimsJws.getBody(); From c443b7aae995eb8a38d79380c59b9ee6eb3cd4fd Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 14 Jul 2023 11:21:17 -0700 Subject: [PATCH 06/14] Fix the suffix of OBO endpoint Signed-off-by: Ryan Liang --- .../org/opensearch/security/filter/SecurityRestFilter.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 622ca46f1d..c21b0d1230 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -89,6 +89,8 @@ public class SecurityRestFilter { private static final String HEALTH_SUFFIX = "health"; private static final String WHO_AM_I_SUFFIX = "whoami"; + private static final String ON_BEHALF_OF_SUFFIX = "onbehalfof"; + private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); @@ -260,8 +262,7 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha } if (HTTPHelper.containsOBOToken(request)) { - String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; - if (request.method() == Method.POST && OBO_ENDPOINT_PREFIX.equals(suffix)) { + if (request.method() == Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); log.error(exception.toString()); auditLog.logBadHeaders(request); From d5ec0abbb200782db95997504a70ca12ef50fceb Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 18 Jul 2023 09:08:49 -0700 Subject: [PATCH 07/14] Add the check for token type and modify the test cases Signed-off-by: Ryan Liang --- .../http/OnBehalfOfAuthenticator.java | 9 ++++- .../security/authtoken/jwt/JwtVendorTest.java | 1 + .../http/OnBehalfOfAuthenticatorTest.java | 37 +++++++++++++------ 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 00f69c55dc..e5c08c477b 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -45,7 +45,8 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private static final Pattern BEARER = Pattern.compile("^\\s*Bearer\\s.*", Pattern.CASE_INSENSITIVE); private static final String BEARER_PREFIX = "bearer "; - private static final String SUBJECT_CLAIM = "sub"; + private static final String TOKEN_TYPE_CLAIM = "typ"; + private static final String TOKEN_TYPE = "obo"; private final JwtParser jwtParser; private final String encryptionKey; @@ -182,6 +183,12 @@ private AuthCredentials extractCredentials0(final RestRequest request) { return null; } + final String tokenType = claims.get(TOKEN_TYPE_CLAIM).toString(); + if (tokenType != TOKEN_TYPE) { + log.error("This toke is not verifying as an on-behalf-of token"); + return null; + } + List roles = extractSecurityRolesFromClaims(claims); String[] backendRoles = extractBackendRolesFromClaims(claims); diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index a278a526b3..60ed365285 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -62,6 +62,7 @@ public void testCreateJwtWithRoles() throws Exception { JwsJwtCompactConsumer jwtConsumer = new JwsJwtCompactConsumer(encodedJwt); JwtToken jwt = jwtConsumer.getJwtToken(); + Assert.assertEquals("obo", jwt.getClaim("typ")); Assert.assertEquals("cluster_0", jwt.getClaim("iss")); Assert.assertEquals("admin", jwt.getClaim("sub")); Assert.assertEquals("audience_0", jwt.getClaim("aud")); diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index d71fe54951..fc1c965cec 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -52,7 +52,7 @@ public void testNoKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( null, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a RuntimeException"); @@ -68,7 +68,7 @@ public void testEmptyKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( null, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a RuntimeException"); @@ -153,6 +153,7 @@ public void testNonSpecifyOBOSetting() throws Exception { public void testBearer() throws Exception { String jwsToken = Jwts.builder() + .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512) @@ -175,6 +176,7 @@ public void testBearer() throws Exception { public void testBearerWrongPosition() throws Exception { String jwsToken = Jwts.builder() + .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") .signWith(secretKey, SignatureAlgorithm.HS512) @@ -192,6 +194,7 @@ public void testBearerWrongPosition() throws Exception { @Test public void testBasicAuthHeader() throws Exception { String jwsToken = Jwts.builder() + .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") .signWith(secretKey, SignatureAlgorithm.HS512) @@ -207,11 +210,10 @@ public void testBasicAuthHeader() throws Exception { @Test public void testRoles() throws Exception { - List roles = List.of("IT", "HR"); final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), true ); @@ -221,13 +223,26 @@ public void testRoles() throws Exception { Assert.assertEquals(0, credentials.getBackendRoles().size()); } + @Test + public void testNoTokenType() throws Exception { + + final AuthCredentials credentials = extractCredentialsFromJwtHeader( + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder().setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), + true + ); + + Assert.assertNull(credentials); + } + @Test public void testNullClaim() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").claim("dr", null).setAudience("svc1"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", null).setAudience("svc1"), false ); @@ -242,7 +257,7 @@ public void testNonStringClaim() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").claim("dr", 123L).setAudience("svc1"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").claim("dr", 123L).setAudience("svc1"), true ); @@ -258,7 +273,7 @@ public void testRolesMissing() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").setAudience("svc1"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy").setAudience("svc1"), false ); @@ -274,7 +289,7 @@ public void testWrongSubjectKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().claim("roles", "role1,role2").claim("asub", "Dr. Who").setAudience("svc1"), + Jwts.builder().claim("typ", "obo").claim("roles", "role1,role2").claim("asub", "Dr. Who").setAudience("svc1"), false ); @@ -287,7 +302,7 @@ public void testExp() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Expired").setExpiration(new Date(100)), + Jwts.builder().claim("typ", "obo").setSubject("Expired").setExpiration(new Date(100)), false ); @@ -300,7 +315,7 @@ public void testNbf() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( signingKeyB64Encoded, claimsEncryptionKey, - Jwts.builder().setSubject("Expired").setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))), + Jwts.builder().claim("typ", "obo").setSubject("Expired").setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))), false ); @@ -311,7 +326,7 @@ public void testNbf() throws Exception { public void testRolesArray() throws Exception { JwtBuilder builder = Jwts.builder() - .setPayload("{" + "\"sub\": \"Cluster_0\"," + "\"aud\": \"ext_0\"," + "\"dr\": \"a,b,3rd\"" + "}"); + .setPayload("{" + "\"typ\": \"obo\"," + "\"sub\": \"Cluster_0\"," + "\"aud\": \"ext_0\"," + "\"dr\": \"a,b,3rd\"" + "}"); final AuthCredentials credentials = extractCredentialsFromJwtHeader(signingKeyB64Encoded, claimsEncryptionKey, builder, true); From 4b095ae8fb3d8d213b83aad71efefc26fb920a73 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 18 Jul 2023 09:47:58 -0700 Subject: [PATCH 08/14] Fix the obo authenticator test Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfAuthenticator.java | 2 +- .../security/http/OnBehalfOfAuthenticatorTest.java | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index e5c08c477b..c789dca6ab 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -184,7 +184,7 @@ private AuthCredentials extractCredentials0(final RestRequest request) { } final String tokenType = claims.get(TOKEN_TYPE_CLAIM).toString(); - if (tokenType != TOKEN_TYPE) { + if (!tokenType.equals(TOKEN_TYPE)) { log.error("This toke is not verifying as an on-behalf-of token"); return null; } diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index fc1c965cec..32861f4132 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -16,7 +16,6 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.crypto.SecretKey; @@ -169,7 +168,7 @@ public void testBearer() throws Exception { Assert.assertEquals("Leonard McCoy", credentials.getUsername()); Assert.assertEquals(0, credentials.getSecurityRoles().size()); Assert.assertEquals(0, credentials.getBackendRoles().size()); - Assert.assertEquals(2, credentials.getAttributes().size()); + Assert.assertEquals(3, credentials.getAttributes().size()); } @Test @@ -227,10 +226,10 @@ public void testRoles() throws Exception { public void testNoTokenType() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( - signingKeyB64Encoded, - claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), - true + signingKeyB64Encoded, + claimsEncryptionKey, + Jwts.builder().setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"), + true ); Assert.assertNull(credentials); From f4410fb0260d957d561d8f7628099bd5144fdf17 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 21 Jul 2023 15:33:58 -0700 Subject: [PATCH 09/14] Move the api suffix checking into the obo authenticator: extractCredential0() Signed-off-by: Ryan Liang --- .../security/filter/SecurityRestFilter.java | 13 ------- .../http/OnBehalfOfAuthenticator.java | 18 +++++++++ .../security/support/HTTPHelper.java | 39 ------------------- 3 files changed, 18 insertions(+), 52 deletions(-) diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index c21b0d1230..9b06e15c5d 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -89,8 +89,6 @@ public class SecurityRestFilter { private static final String HEALTH_SUFFIX = "health"; private static final String WHO_AM_I_SUFFIX = "whoami"; - private static final String ON_BEHALF_OF_SUFFIX = "onbehalfof"; - private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); @@ -260,17 +258,6 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha ); } } - - if (HTTPHelper.containsOBOToken(request)) { - if (request.method() == Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { - final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); - log.error(exception.toString()); - auditLog.logBadHeaders(request); - channel.sendResponse(new BytesRestResponse(channel, RestStatus.FORBIDDEN, exception)); - return true; - } - } - return false; } diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index c789dca6ab..e055c0a754 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map.Entry; import java.util.Objects; +import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -28,6 +29,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchException; import org.opensearch.OpenSearchSecurityException; import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Settings; @@ -36,11 +38,19 @@ import org.opensearch.rest.RestRequest; import org.opensearch.security.auth.HTTPAuthenticator; import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil; +import org.opensearch.security.ssl.util.ExceptionUtils; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.keyUtil; +import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; +import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; + public class OnBehalfOfAuthenticator implements HTTPAuthenticator { + private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; + private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); + private static final String ON_BEHALF_OF_SUFFIX = "onbehalfof"; + protected final Logger log = LogManager.getLogger(this.getClass()); private static final Pattern BEARER = Pattern.compile("^\\s*Bearer\\s.*", Pattern.CASE_INSENSITIVE); @@ -194,6 +204,14 @@ private AuthCredentials extractCredentials0(final RestRequest request) { final AuthCredentials ac = new AuthCredentials(subject, roles, backendRoles).markComplete(); + Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); + final String suffix = matcher.matches() ? matcher.group(2) : null; + if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { + final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); + log.error(exception.toString()); + return null; + } + for (Entry claim : claims.entrySet()) { ac.addAttribute("attr.jwt." + claim.getKey(), String.valueOf(claim.getValue())); } diff --git a/src/main/java/org/opensearch/security/support/HTTPHelper.java b/src/main/java/org/opensearch/security/support/HTTPHelper.java index b18bad83e6..c3b191f770 100644 --- a/src/main/java/org/opensearch/security/support/HTTPHelper.java +++ b/src/main/java/org/opensearch/security/support/HTTPHelper.java @@ -31,9 +31,6 @@ import java.util.List; import java.util.Map; -import io.jsonwebtoken.Claims; -import io.jsonwebtoken.Jws; -import io.jsonwebtoken.Jwts; import org.apache.logging.log4j.Logger; import org.opensearch.rest.RestRequest; @@ -103,40 +100,4 @@ public static boolean containsBadHeader(final RestRequest request) { return false; } - - public static boolean containsOBOToken(final RestRequest request) { - final Map> headers; - - if (request != null && (headers = request.getHeaders()) != null) { - List authHeaders = headers.get("Authorization"); - if (authHeaders != null && !authHeaders.isEmpty()) { - // Iterate through the list of 'Authorization' headers, checking each for the 'Bearer' prefix. - for (String authHeader : authHeaders) { - if (authHeader != null && authHeader.startsWith("Bearer ")) { - // Header found, extract the token to verify it's an OBO token. - String token = authHeader.substring("Bearer ".length()); - if (isOBOToken(token)) { - return true; - } - } - } - } - } - - return false; - } - - private static boolean isOBOToken(String token) { - String tokenIdentifierClaimKey = "typ"; - String tokenIdentifier = "obo"; - - Jws claimsJws = Jwts.parserBuilder().build().parseClaimsJws(token); - Claims claims = claimsJws.getBody(); - - if (claims.containsKey(tokenIdentifierClaimKey) && tokenIdentifier.equals(claims.get(tokenIdentifierClaimKey))) { - return true; - } - return false; - } - } From ad9b6bf4b4370f0ecccc09c0a21d6b7fd302bc77 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 4 Aug 2023 09:04:23 -0700 Subject: [PATCH 10/14] Adapt the transport library refactoring Signed-off-by: Ryan Liang --- .../security/action/onbehalf/CreateOnBehalfOfTokenAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 be12de5bae..69c51482b2 100644 --- a/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java +++ b/src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java @@ -24,7 +24,7 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; -import org.opensearch.common.transport.TransportAddress; +import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; From a2a32d7bce1583442bab21b7531ed0b8bddb181b Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 4 Aug 2023 12:46:42 -0700 Subject: [PATCH 11/14] Fix the tests and polish the edge case of using obo to access obo endpoint Signed-off-by: Ryan Liang --- .../http/OnBehalfOfJwtAuthenticationTest.java | 5 +++-- .../test/framework/cluster/TestRestClient.java | 10 ++++++++-- .../security/http/OnBehalfOfAuthenticator.java | 18 +++++++++--------- .../http/OnBehalfOfAuthenticatorTest.java | 4 +++- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 0ef08fd2a8..5008f5d5a0 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -58,6 +58,7 @@ public class OnBehalfOfJwtAuthenticationTest { public static final String DEFAULT_PASSWORD = "secret"; public static final String OBO_TOKEN_REASON = "{\"reason\":\"Test generation\"}"; public static final String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; + public static final String OBO_REASON = "{\"reason\":\"Testing\", \"service\":\"extension123\"}"; @ClassRule public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) @@ -96,8 +97,8 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() { Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { - TestRestClient.HttpResponse response = client.getOBOToken(adminOboAuthHeader); - response.assertStatusCode(403); + TestRestClient.HttpResponse response = client.getOBOTokenFromOboEndpoint(OBO_REASON, adminOboAuthHeader); + response.assertStatusCode(401); } } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index 0d343c06bd..4b5e21032d 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -135,8 +135,14 @@ public HttpResponse getAuthInfo(Header... headers) { return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers); } - public HttpResponse getOBOToken(Header... headers) { - return executeRequest(new HttpPost(getHttpServerUri() + "/_plugin/_security/api/user/onbehalfof?pretty"), headers); + public HttpResponse getOBOTokenFromOboEndpoint(String jsonData, Header... headers) { + try { + HttpPost httpPost = new HttpPost(new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/user/onbehalfof?pretty").build()); + httpPost.setEntity(toStringEntity(jsonData)); + return executeRequest(httpPost, mergeHeaders(CONTENT_TYPE_JSON, headers)); + } catch (URISyntaxException ex) { + throw new RuntimeException("Incorrect URI syntax", ex); + } } public void assertCorrectCredentials(String expectedUserName) { diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index e055c0a754..233c915b1a 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -49,7 +49,7 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); - private static final String ON_BEHALF_OF_SUFFIX = "onbehalfof"; + private static final String ON_BEHALF_OF_SUFFIX = "api/user/onbehalfof"; protected final Logger log = LogManager.getLogger(this.getClass()); @@ -179,6 +179,14 @@ private AuthCredentials extractCredentials0(final RestRequest request) { } try { + Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); + final String suffix = matcher.matches() ? matcher.group(2) : null; + if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { + final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); + log.error(exception.toString()); + return null; + } + final Claims claims = jwtParser.parseClaimsJws(jwtToken).getBody(); final String subject = claims.getSubject(); @@ -204,14 +212,6 @@ private AuthCredentials extractCredentials0(final RestRequest request) { final AuthCredentials ac = new AuthCredentials(subject, roles, backendRoles).markComplete(); - Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); - final String suffix = matcher.matches() ? matcher.group(2) : null; - if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { - final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); - log.error(exception.toString()); - return null; - } - for (Entry claim : claims.entrySet()) { ac.addAttribute("attr.jwt." + claim.getKey(), String.valueOf(claim.getValue())); } diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java index 32861f4132..e9208ccac8 100644 --- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java @@ -83,7 +83,7 @@ public void testBadKey() throws Exception { final AuthCredentials credentials = extractCredentialsFromJwtHeader( BaseEncoding.base64().encode(new byte[] { 1, 3, 3, 4, 3, 6, 7, 8, 3, 10 }), claimsEncryptionKey, - Jwts.builder().setSubject("Leonard McCoy"), + Jwts.builder().claim("typ", "obo").setSubject("Leonard McCoy"), false ); Assert.fail("Expected a WeakKeyException"); @@ -119,6 +119,7 @@ public void testInvalid() throws Exception { @Test public void testDisabled() throws Exception { String jwsToken = Jwts.builder() + .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512) @@ -135,6 +136,7 @@ public void testDisabled() throws Exception { @Test public void testNonSpecifyOBOSetting() throws Exception { String jwsToken = Jwts.builder() + .claim("typ", "obo") .setSubject("Leonard McCoy") .setAudience("ext_0") .signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512) From 7ef3edf17883494c9b1399734ef62c268da6c4e0 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 4 Aug 2023 15:51:47 -0700 Subject: [PATCH 12/14] Block account endpoint + tests Signed-off-by: Ryan Liang --- .../http/OnBehalfOfJwtAuthenticationTest.java | 13 +++++++++++++ .../test/framework/cluster/TestRestClient.java | 11 +++++++++++ .../security/http/OnBehalfOfAuthenticator.java | 4 +++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 5008f5d5a0..661b21e739 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -56,9 +56,11 @@ public class OnBehalfOfJwtAuthenticationTest { private static final String encryptionKey = Base64.getEncoder().encodeToString("encryptionKey".getBytes(StandardCharsets.UTF_8)); public static final String ADMIN_USER_NAME = "admin"; public static final String DEFAULT_PASSWORD = "secret"; + public static final String NEW_PASSWORD = "testPassword123!!"; public static final String OBO_TOKEN_REASON = "{\"reason\":\"Test generation\"}"; public static final String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; public static final String OBO_REASON = "{\"reason\":\"Testing\", \"service\":\"extension123\"}"; + public static final String CURRENT_AND_NEW_PASSWORDS = "{ \"current_password\": \"" + DEFAULT_PASSWORD + "\", \"password\": \"" + NEW_PASSWORD + "\" }"; @ClassRule public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) @@ -102,6 +104,17 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() { } } + @Test + public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() { + String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); + Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); + + try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { + TestRestClient.HttpResponse response = client.changeInternalUserPassword(CURRENT_AND_NEW_PASSWORDS, adminOboAuthHeader); + response.assertStatusCode(401); + } + } + private String generateOboToken(String username, String password) { try (TestRestClient client = cluster.getRestClient(username, password)) { client.assertCorrectCredentials(username); diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index 4b5e21032d..f5ab6008a9 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -62,6 +62,7 @@ import org.apache.hc.client5.http.routing.HttpRoutePlanner; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.NameValuePair; import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicHeader; @@ -145,6 +146,16 @@ public HttpResponse getOBOTokenFromOboEndpoint(String jsonData, Header... header } } + public HttpResponse changeInternalUserPassword(String jsonData, Header...headers) { + try { + HttpPut httpPut = new HttpPut(new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/account?pretty").build()); + httpPut.setEntity(toStringEntity(jsonData)); + return executeRequest(httpPut, mergeHeaders(CONTENT_TYPE_JSON, headers)); + } catch (URISyntaxException ex) { + throw new RuntimeException("Incorrect URI syntax", ex); + } + } + public void assertCorrectCredentials(String expectedUserName) { HttpResponse response = getAuthInfo(); assertThat(response, notNullValue()); diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java index 233c915b1a..f812cc97ee 100644 --- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java @@ -50,6 +50,7 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator { private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)"; private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); private static final String ON_BEHALF_OF_SUFFIX = "api/user/onbehalfof"; + private static final String ACCOUNT_SUFFIX = "api/account"; protected final Logger log = LogManager.getLogger(this.getClass()); @@ -181,7 +182,8 @@ private AuthCredentials extractCredentials0(final RestRequest request) { try { Matcher matcher = PATTERN_PATH_PREFIX.matcher(request.path()); final String suffix = matcher.matches() ? matcher.group(2) : null; - if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix)) { + if (request.method() == RestRequest.Method.POST && ON_BEHALF_OF_SUFFIX.equals(suffix) + || request.method() == RestRequest.Method.PUT && ACCOUNT_SUFFIX.equals(suffix)) { final OpenSearchException exception = ExceptionUtils.invalidUsageOfOBOTokenException(); log.error(exception.toString()); return null; From b837cb9cb23233fc0279737733368fee4f35cd25 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 4 Aug 2023 15:59:38 -0700 Subject: [PATCH 13/14] Fix code ql Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfJwtAuthenticationTest.java | 2 +- .../org/opensearch/test/framework/cluster/TestRestClient.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 661b21e739..fbcd125f2d 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -59,7 +59,7 @@ public class OnBehalfOfJwtAuthenticationTest { public static final String NEW_PASSWORD = "testPassword123!!"; public static final String OBO_TOKEN_REASON = "{\"reason\":\"Test generation\"}"; public static final String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; - public static final String OBO_REASON = "{\"reason\":\"Testing\", \"service\":\"extension123\"}"; + public static final String OBO_REASON = "{\"reason\":\"Testing\", \"service\":\"self-issued\"}"; public static final String CURRENT_AND_NEW_PASSWORDS = "{ \"current_password\": \"" + DEFAULT_PASSWORD + "\", \"password\": \"" + NEW_PASSWORD + "\" }"; @ClassRule diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index f5ab6008a9..d6d3ad3953 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -62,7 +62,6 @@ import org.apache.hc.client5.http.routing.HttpRoutePlanner; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.NameValuePair; import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicHeader; From 082874a4f122b8f17b6f5dbd989fdb1c15b61a40 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 4 Aug 2023 16:16:04 -0700 Subject: [PATCH 14/14] Fix lint again Signed-off-by: Ryan Liang --- .../security/http/OnBehalfOfJwtAuthenticationTest.java | 6 +++++- .../opensearch/test/framework/cluster/TestRestClient.java | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index fbcd125f2d..4e90044ec4 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -60,7 +60,11 @@ public class OnBehalfOfJwtAuthenticationTest { public static final String OBO_TOKEN_REASON = "{\"reason\":\"Test generation\"}"; public static final String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/user/onbehalfof"; public static final String OBO_REASON = "{\"reason\":\"Testing\", \"service\":\"self-issued\"}"; - public static final String CURRENT_AND_NEW_PASSWORDS = "{ \"current_password\": \"" + DEFAULT_PASSWORD + "\", \"password\": \"" + NEW_PASSWORD + "\" }"; + public static final String CURRENT_AND_NEW_PASSWORDS = "{ \"current_password\": \"" + + DEFAULT_PASSWORD + + "\", \"password\": \"" + + NEW_PASSWORD + + "\" }"; @ClassRule public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index d6d3ad3953..e48acafd2f 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -145,7 +145,7 @@ public HttpResponse getOBOTokenFromOboEndpoint(String jsonData, Header... header } } - public HttpResponse changeInternalUserPassword(String jsonData, Header...headers) { + public HttpResponse changeInternalUserPassword(String jsonData, Header... headers) { try { HttpPut httpPut = new HttpPut(new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/account?pretty").build()); httpPut.setEntity(toStringEntity(jsonData));