From ccc3e34e29b32d7517ca88f552b53f995d090a02 Mon Sep 17 00:00:00 2001
From: Ryan Liang <109499885+RyanL1997@users.noreply.github.com>
Date: Tue, 24 Oct 2023 09:41:31 -0700
Subject: [PATCH] [Forwardport main] Switch to `supportsImpersonation` check
 for http auth backend and add Privileged Action for `JwtParserBuilder `
 (#3579)

### Description
Switch to `supportsImpersonation` check for http auth backend + wrap
JwtParserBuilder with `doPrivileged`

Reference to @cwperks's
[comment](https://github.com/opensearch-project/security/pull/3563#discussion_r1365539239):
>As a default implementation the authDomain could have:
>
>```
>default boolean supportsImpersonation() { return true; }
>```
>
>and any authDomain that does not support it can override:
>
>```
>@Override
>public boolean supportsImpersonation() { return false; }
>```

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)
Enhancement

### Issues Resolved
* Related https://github.com/opensearch-project/security/pull/3563

Is this a backport? If so, please add backport PR # and/or commits #
It has already been included in `2.x`

### Check List
- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Ryan Liang <jiallian@amazon.com>
---
 .../security/auth/BackendRegistry.java         |  4 +---
 .../security/auth/HTTPAuthenticator.java       |  9 +++++++++
 .../security/http/OnBehalfOfAuthenticator.java | 18 ++++++++++++++++--
 .../http/OnBehalfOfAuthenticatorTest.java      |  2 +-
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java
index c500124129..a064207964 100644
--- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java
+++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java
@@ -61,7 +61,6 @@
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannel;
 import org.opensearch.security.filter.SecurityResponse;
-import org.opensearch.security.http.OnBehalfOfAuthenticator;
 import org.opensearch.security.http.XFFResolver;
 import org.opensearch.security.securityconf.DynamicConfigModel;
 import org.opensearch.security.support.ConfigConstants;
@@ -619,8 +618,7 @@ private User impersonate(final SecurityRequest request, final User originalUser)
             for (final AuthDomain authDomain : restAuthDomains) {
                 final AuthenticationBackend authenticationBackend = authDomain.getBackend();
 
-                // Skip over the OnBehalfOfAuthenticator since it is not compatible for user impersonation
-                if (authDomain.getHttpAuthenticator() instanceof OnBehalfOfAuthenticator) {
+                if (!authDomain.getHttpAuthenticator().supportsImpersonation()) {
                     continue;
                 }
 
diff --git a/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java b/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java
index f5a4df34b5..c79576ef5f 100644
--- a/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java
+++ b/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java
@@ -83,4 +83,13 @@ public interface HTTPAuthenticator {
      * @return Optional response if is not supported/necessary, response object otherwise.
      */
     Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials credentials);
+
+    /**
+     * Indicates whether this authenticator supports user impersonation.
+     *
+     * @return true if impersonation is supported, false otherwise.
+     */
+    default boolean supportsImpersonation() {
+        return true;
+    }
 }
diff --git a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
index 4ac3be335f..a3d3dec710 100644
--- a/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
+++ b/src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
@@ -67,8 +67,18 @@ public OnBehalfOfAuthenticator(Settings settings, String clusterName) {
         String oboEnabledSetting = settings.get("enabled", "true");
         oboEnabled = Boolean.parseBoolean(oboEnabledSetting);
         encryptionKey = settings.get("encryption_key");
-        JwtParserBuilder builder = initParserBuilder(settings.get("signing_key"));
-        jwtParser = builder.build();
+
+        final SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            sm.checkPermission(new SpecialPermission());
+        }
+        jwtParser = AccessController.doPrivileged(new PrivilegedAction<JwtParser>() {
+            @Override
+            public JwtParser run() {
+                JwtParserBuilder builder = initParserBuilder(settings.get("signing_key"));
+                return builder.build();
+            }
+        });
 
         this.clusterName = clusterName;
         this.encryptionUtil = new EncryptionDecryptionUtil(encryptionKey);
@@ -244,4 +254,8 @@ public String getType() {
         return "onbehalfof_jwt";
     }
 
+    @Override
+    public boolean supportsImpersonation() {
+        return false;
+    }
 }
diff --git a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
index b32792190f..478e59ac13 100644
--- a/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
+++ b/src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
@@ -349,7 +349,7 @@ public void testSecurityManagerCheck() {
             System.setSecurityManager(null);
         }
 
-        verify(mockSecurityManager, times(2)).checkPermission(any(SpecialPermission.class));
+        verify(mockSecurityManager, times(3)).checkPermission(any(SpecialPermission.class));
     }
 
     @Test