Skip to content

Commit

Permalink
[Forwardport main] Switch to supportsImpersonation check for http a…
Browse files Browse the repository at this point in the history
…uth 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](#3563 (comment)):
>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 #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>
  • Loading branch information
RyanL1997 committed Oct 24, 2023
1 parent 59736c5 commit ccc3e34
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -244,4 +254,8 @@ public String getType() {
return "onbehalfof_jwt";
}

@Override
public boolean supportsImpersonation() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ccc3e34

Please sign in to comment.