[Feature/Extension] Restrict OBO token's usage for certain endpoints (opensearch-project#3008)

RyanL1997 · web-flow · commit d643fb285fa7 · 2023-08-04T22:57:18.000-07:00
Signed-off-by: Ryan Liang &lt;jiallian@amazon.com&gt;
diff --git 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;
@@ -55,8 +56,15 @@ 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\":\"self-issued\"}";
+    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)
@@ -76,60 +84,62 @@ 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<String, Object> oboEndPointResponse = response.getBodyAs(Map.class);
-            assertThat(oboEndPointResponse, allOf(aMapWithSize(3), hasKey("user"), hasKey("onBehalfOfToken"), hasKey("duration")));
+        String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD);
+        Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken);
+        authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, 200);
+    }
 
-            String encodedOboTokenStr = oboEndPointResponse.get("onBehalfOfToken").toString();
+    @Test
+    public void shouldNotAuthenticateWithATemperedOBOToken() {
+        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);
+    }
 
-            adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + encodedOboTokenStr);
-        }
+    @Test
+    public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() {
+        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.getAuthInfo();
-            response.assertStatusCode(200);
-
-            String username = response.getTextFromJsonBody(POINTER_USERNAME);
-            assertThat(username, equalTo(ADMIN_USER_NAME));
+            TestRestClient.HttpResponse response = client.getOBOTokenFromOboEndpoint(OBO_REASON, adminOboAuthHeader);
+            response.assertStatusCode(401);
         }
     }
 
     @Test
-    public void shouldNotAuthenticateWithATemperedOBOToken() {
-        Header adminOboAuthHeader;
+    public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() {
+        String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD);
+        Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken);
 
-        try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
-
-            client.assertCorrectCredentials(ADMIN_USER_NAME);
+        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);
             TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON);
             response.assertStatusCode(200);
-
             Map<String, Object> 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
@@ -135,6 +135,26 @@ public HttpResponse getAuthInfo(Header... headers) {
         return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?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 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/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;
diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java
@@ -105,13 +105,16 @@ public String createJwt(
         List<String> roles,
         List<String> backendRoles
     ) throws Exception {
+        String tokenIdentifier = "obo";
         long timeMillis = timeProvider.getAsLong();
         Instant now = Instant.ofEpochMilli(timeProvider.getAsLong());
 
         jwtProducer.setSignatureProvider(JwsUtils.getSignatureProvider(signingKey));
         JwtClaims jwtClaims = new JwtClaims();
         JwtToken jwt = new JwtToken(jwtClaims);
 
+        jwtClaims.setProperty("typ", 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
@@ -258,7 +258,6 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha
                 );
             }
         }
-
         return false;
     }
 
diff --git 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,16 +38,26 @@
 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 = "api/user/onbehalfof";
+    private static final String ACCOUNT_SUFFIX = "api/account";
+
     protected final Logger log = LogManager.getLogger(this.getClass());
 
     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;
@@ -168,6 +180,15 @@ 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)
+                || request.method() == RestRequest.Method.PUT && ACCOUNT_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();
@@ -182,6 +203,12 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
                 return null;
             }
 
+            final String tokenType = claims.get(TOKEN_TYPE_CLAIM).toString();
+            if (!tokenType.equals(TOKEN_TYPE)) {
+                log.error("This toke is not verifying as an on-behalf-of token");
+                return null;
+            }
+
             List<String> roles = extractSecurityRolesFromClaims(claims);
             String[] backendRoles = extractBackendRolesFromClaims(claims);
 
diff --git 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-Of Token is not allowed to be used for accessing this endopoint.");
+    }
+
     public static OpenSearchException createTransportClientNoLongerSupportedException() {
         return new OpenSearchException("Transport client authentication no longer supported.");
     }
diff --git 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

Original file line number	Diff line number	Diff line change
`@@ -258,7 +258,6 @@ private boolean checkAndAuthenticateRequest(RestRequest request, RestChannel cha`
`258`	`258`	`);`
`259`	`259`	`}`
`260`	`260`	`}`
`261`		`-`
`262`	`261`	`return false;`
`263`	`262`	`}`
`264`	`263`
Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,10 @@ public static OpenSearchException createBadHeaderException() {`
`64`	`64`	`);`
`65`	`65`	`}`
`66`	`66`
	`67`	`+ public static OpenSearchException invalidUsageOfOBOTokenException() {`
	`68`	`+ return new OpenSearchException("On-Behalf-Of Token is not allowed to be used for accessing this endopoint.");`
	`69`	`+ }`
	`70`	`+`
`67`	`71`	`public static OpenSearchException createTransportClientNoLongerSupportedException() {`
`68`	`72`	`return new OpenSearchException("Transport client authentication no longer supported.");`
`69`	`73`	`}`