Fix the tests and polish the edge case of using obo to access obo end…

…point Signed-off-by: Ryan Liang <jiallian@amazon.com>
RyanL1997 · Aug 4, 2023 · a2a32d7 · a2a32d7
1 parent ad9b6bf
commit a2a32d7
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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<String, Object> 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
@@ -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)