Skip to content

Commit

Permalink
Work around for not set static cluster service
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryan Liang <jiallian@amazon.com>
  • Loading branch information
@RyanL1997
RyanL1997 committed Aug 22, 2023
1 parent a4efad6 commit 3ebff2b
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 68 deletions.
12 changes: 2 additions & 10 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile ThreadPool threadPool;
private volatile ConfigurationRepository cr;
private volatile AdminDNs adminDns;
private static volatile ClusterService cs;
private volatile ClusterService cs;
private volatile AtomicReference<DiscoveryNode> localNode = new AtomicReference<>();
private volatile AuditLog auditLog;
private volatile BackendRegistry backendRegistry;
Expand Down Expand Up @@ -1049,7 +1049,7 @@ public Collection<Object> createComponents(
configPath,
compatConfig
);
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih);
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, this.cs.getClusterName().value());
dcf.registerDCFListener(backendRegistry);
dcf.registerDCFListener(compatConfig);
dcf.registerDCFListener(irr);
Expand Down Expand Up @@ -1985,12 +1985,4 @@ public void start() {}
public void stop() {}

}

public static void setClusterService(ClusterService clusterService) {
cs = clusterService;
}

public static ClusterService getClusterName() {
return cs;
}
}
6 changes: 3 additions & 3 deletions src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestRequest;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil;
import org.opensearch.security.ssl.util.ExceptionUtils;
Expand All @@ -63,12 +62,14 @@ public class OnBehalfOfAuthenticator implements HTTPAuthenticator {
private final JwtParser jwtParser;
private final String encryptionKey;
private final Boolean oboEnabled;
private final String clusterName;

public OnBehalfOfAuthenticator(Settings settings) {
public OnBehalfOfAuthenticator(Settings settings, String clusterName) {
String oboEnabledSetting = settings.get("enabled");
oboEnabled = oboEnabledSetting == null ? Boolean.TRUE : Boolean.valueOf(oboEnabledSetting);
encryptionKey = settings.get("encryption_key");
jwtParser = initParser(settings.get("signing_key"));
this.clusterName = clusterName;
}

private JwtParser initParser(final String signingKey) {
Expand Down Expand Up @@ -200,7 +201,6 @@ private AuthCredentials extractCredentials0(final RestRequest request) {
}

final String issuer = claims.getIssuer();
final String clusterName = OpenSearchSecurityPlugin.getClusterName().getClusterName().value();
if (!issuer.equals(clusterName)) {
log.error("This issuer of this OBO does not match the current cluster identifier");
return null;
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public class DynamicConfigFactory implements Initializable, ConfigurationChangeL
private static SecurityDynamicConfiguration<TenantV7> staticTenants = SecurityDynamicConfiguration.empty();
private static final WhitelistingSettings defaultWhitelistingSettings = new WhitelistingSettings();
private static final AllowlistingSettings defaultAllowlistingSettings = new AllowlistingSettings();
private final String clusterName;

static void resetStatics() {
staticRoles = SecurityDynamicConfiguration.empty();
Expand Down Expand Up @@ -137,12 +138,14 @@ public DynamicConfigFactory(
final Path configPath,
Client client,
ThreadPool threadPool,
ClusterInfoHolder cih
ClusterInfoHolder cih,
String clusterName
) {
super();
this.cr = cr;
this.opensearchSettings = opensearchSettings;
this.configPath = configPath;
this.clusterName = clusterName;

if (opensearchSettings.getAsBoolean(ConfigConstants.SECURITY_UNSUPPORTED_LOAD_STATIC_RESOURCES, true)) {
try {
Expand Down Expand Up @@ -271,7 +274,7 @@ public void onChange(Map<CType, SecurityDynamicConfiguration<?>> typeToConfig) {
);

// rebuild v7 Models
dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab);
dcm = new DynamicConfigModelV7(getConfigV7(config), opensearchSettings, configPath, iab, this.clusterName);
ium = new InternalUsersModelV7(
(SecurityDynamicConfiguration<InternalUserV7>) internalusers,
(SecurityDynamicConfiguration<RoleV7>) roles,
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,21 @@ public class DynamicConfigModelV7 extends DynamicConfigModel {
private Multimap<String, AuthFailureListener> authBackendFailureListeners;
private List<ClientBlockRegistry<InetAddress>> ipClientBlockRegistries;
private Multimap<String, ClientBlockRegistry<String>> authBackendClientBlockRegistries;

public DynamicConfigModelV7(ConfigV7 config, Settings opensearchSettings, Path configPath, InternalAuthenticationBackend iab) {
private String clusterName;

public DynamicConfigModelV7(
ConfigV7 config,
Settings opensearchSettings,
Path configPath,
InternalAuthenticationBackend iab,
String clusterName
) {
super();
this.config = config;
this.opensearchSettings = opensearchSettings;
this.configPath = configPath;
this.iab = iab;
this.clusterName = clusterName;
buildAAA();
}

Expand Down Expand Up @@ -371,7 +379,7 @@ private void buildAAA() {
if (oboSettings.get("signing_key") != null && oboSettings.get("encryption_key") != null) {
final AuthDomain _ad = new AuthDomain(
new NoOpAuthenticationBackend(Settings.EMPTY, null),
new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings()),
new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.clusterName),
false,
-1
);
Expand Down
76 changes: 26 additions & 50 deletions src/test/java/org/opensearch/security/http/OnBehalfOfAuthenticatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,15 @@
import io.jsonwebtoken.security.Keys;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.hc.core5.http.HttpHeaders;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import org.opensearch.cluster.ClusterName;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.user.AuthCredentials;
import org.opensearch.security.util.FakeRestRequest;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class OnBehalfOfAuthenticatorTest {
final static String clusterNameString = "cluster_0";
final static String clusterName = "cluster_0";
final static String enableOBO = "true";
final static String disableOBO = "false";
final static String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16);
Expand All @@ -53,28 +45,14 @@ public class OnBehalfOfAuthenticatorTest {
final static String signingKeyB64Encoded = BaseEncoding.base64().encode(signingKey.getBytes(StandardCharsets.UTF_8));
final static SecretKey secretKey = Keys.hmacShaKeyFor(signingKeyB64Encoded.getBytes(StandardCharsets.UTF_8));

@Before
public void setupMocks() {
ClusterService mockedClusterService = mock(ClusterService.class);
ClusterName mockedClusterName = new ClusterName(clusterNameString);
when(mockedClusterService.getClusterName()).thenReturn(mockedClusterName);

OpenSearchSecurityPlugin.setClusterService(mockedClusterService);
}

@After
public void tearDown() {
OpenSearchSecurityPlugin.setClusterService(null);
}

@Test
public void testNoKey() throws Exception {

try {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
null,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy"),
false
);
Assert.fail("Expected a RuntimeException");
Expand All @@ -90,7 +68,7 @@ public void testEmptyKey() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
null,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy"),
false
);
Assert.fail("Expected a RuntimeException");
Expand All @@ -106,7 +84,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().setIssuer(clusterNameString).setSubject("Leonard McCoy"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy"),
false
);
Assert.fail("Expected a WeakKeyException");
Expand All @@ -118,7 +96,7 @@ public void testBadKey() throws Exception {
@Test
public void testTokenMissing() throws Exception {

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);
Map<String, String> headers = new HashMap<String, String>();

AuthCredentials credentials = jwtAuth.extractCredentials(new FakeRestRequest(headers, new HashMap<String, String>()), null);
Expand All @@ -131,7 +109,7 @@ public void testInvalid() throws Exception {

String jwsToken = "123invalidtoken..";

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

Expand All @@ -142,13 +120,13 @@ public void testInvalid() throws Exception {
@Test
public void testDisabled() throws Exception {
String jwsToken = Jwts.builder()
.setIssuer(clusterNameString)
.setIssuer(clusterName)
.setSubject("Leonard McCoy")
.setAudience("ext_0")
.signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
.compact();

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(disableOBOSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(disableOBOSettings(), clusterName);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

Expand All @@ -159,13 +137,13 @@ public void testDisabled() throws Exception {
@Test
public void testNonSpecifyOBOSetting() throws Exception {
String jwsToken = Jwts.builder()
.setIssuer(clusterNameString)
.setIssuer(clusterName)
.setSubject("Leonard McCoy")
.setAudience("ext_0")
.signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
.compact();

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(nonSpecifyOBOSetting());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(nonSpecifyOBOSetting(), clusterName);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

Expand All @@ -177,13 +155,13 @@ public void testNonSpecifyOBOSetting() throws Exception {
public void testBearer() throws Exception {

String jwsToken = Jwts.builder()
.setIssuer(clusterNameString)
.setIssuer(clusterName)
.setSubject("Leonard McCoy")
.setAudience("ext_0")
.signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
.compact();

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

Expand All @@ -200,12 +178,12 @@ public void testBearer() throws Exception {
public void testBearerWrongPosition() throws Exception {

String jwsToken = Jwts.builder()
.setIssuer(clusterNameString)
.setIssuer(clusterName)
.setSubject("Leonard McCoy")
.setAudience("ext_0")
.signWith(secretKey, SignatureAlgorithm.HS512)
.compact();
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);

Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", jwsToken + "Bearer " + " 123");
Expand All @@ -218,12 +196,12 @@ public void testBearerWrongPosition() throws Exception {
@Test
public void testBasicAuthHeader() throws Exception {
String jwsToken = Jwts.builder()
.setIssuer(clusterNameString)
.setIssuer(clusterName)
.setSubject("Leonard McCoy")
.setAudience("ext_0")
.signWith(secretKey, SignatureAlgorithm.HS512)
.compact();
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);

Map<String, String> headers = Collections.singletonMap(HttpHeaders.AUTHORIZATION, "Basic " + jwsToken);

Expand All @@ -237,7 +215,7 @@ public void testRoles() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy").claim("dr", "role1,role2").setAudience("svc1"),
true
);

Expand All @@ -253,7 +231,7 @@ public void testNullClaim() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy").claim("dr", null).setAudience("svc1"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy").claim("dr", null).setAudience("svc1"),
false
);

Expand All @@ -268,7 +246,7 @@ public void testNonStringClaim() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy").claim("dr", 123L).setAudience("svc1"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy").claim("dr", 123L).setAudience("svc1"),
true
);

Expand All @@ -284,7 +262,7 @@ public void testRolesMissing() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Leonard McCoy").setAudience("svc1"),
Jwts.builder().setIssuer(clusterName).setSubject("Leonard McCoy").setAudience("svc1"),
false
);

Expand All @@ -300,7 +278,7 @@ public void testWrongSubjectKey() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).claim("roles", "role1,role2").claim("asub", "Dr. Who").setAudience("svc1"),
Jwts.builder().setIssuer(clusterName).claim("roles", "role1,role2").claim("asub", "Dr. Who").setAudience("svc1"),
false
);

Expand All @@ -313,7 +291,7 @@ public void testExp() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder().setIssuer(clusterNameString).setSubject("Expired").setExpiration(new Date(100)),
Jwts.builder().setIssuer(clusterName).setSubject("Expired").setExpiration(new Date(100)),
false
);

Expand All @@ -326,10 +304,7 @@ public void testNbf() throws Exception {
final AuthCredentials credentials = extractCredentialsFromJwtHeader(
signingKeyB64Encoded,
claimsEncryptionKey,
Jwts.builder()
.setIssuer(clusterNameString)
.setSubject("Expired")
.setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))),
Jwts.builder().setIssuer(clusterName).setSubject("Expired").setNotBefore(new Date(System.currentTimeMillis() + (1000 * 36000))),
false
);

Expand Down Expand Up @@ -370,7 +345,7 @@ public void testDifferentIssuer() throws Exception {
.signWith(Keys.hmacShaKeyFor(Base64.getDecoder().decode(signingKeyB64Encoded)), SignatureAlgorithm.HS512)
.compact();

OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings());
OnBehalfOfAuthenticator jwtAuth = new OnBehalfOfAuthenticator(defaultSettings(), clusterName);
Map<String, String> headers = new HashMap<String, String>();
headers.put("Authorization", "Bearer " + jwsToken);

Expand All @@ -391,7 +366,8 @@ private AuthCredentials extractCredentialsFromJwtHeader(
.put("enabled", enableOBO)
.put("signing_key", signingKeyB64Encoded)
.put("encryption_key", encryptionKey)
.build()
.build(),
clusterName
);

final String jwsToken = jwtBuilder.signWith(
Expand Down

0 comments on commit 3ebff2b

Please sign in to comment.