Skip to content

Remove Unused CS Listener from SecurityServerTransportInterceptor #83556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class LocalStateIdentityProviderPlugin extends LocalStateCompositeXPackPl
public LocalStateIdentityProviderPlugin(Settings settings, Path configPath) throws Exception {
super(settings, configPath);
LocalStateIdentityProviderPlugin thisVar = this;
plugins.add(new Security(settings, configPath) {
plugins.add(new Security(settings) {
@Override
protected SSLService getSslService() {
return thisVar.getSslService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected XPackLicenseState getLicenseState() {
return thisVar.getLicenseState();
}
});
plugins.add(new Security(settings, configPath) {
plugins.add(new Security(settings) {
@Override
protected SSLService getSslService() {
return thisVar.getSslService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@
import org.elasticsearch.xpack.security.transport.nio.SecurityNioTransport;

import java.io.IOException;
import java.nio.file.Path;
import java.time.Clock;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -471,11 +470,11 @@ public class Security extends Plugin
private final SetOnce<Transport> transportReference = new SetOnce<>();
private final SetOnce<ScriptService> scriptServiceReference = new SetOnce<>();

public Security(Settings settings, final Path configPath) {
this(settings, configPath, Collections.emptyList());
public Security(Settings settings) {
this(settings, Collections.emptyList());
}

Security(Settings settings, final Path configPath, List<SecurityExtension> extensions) {
Security(Settings settings, List<SecurityExtension> extensions) {
// TODO This is wrong. Settings can change after this. We should use the settings from createComponents
this.settings = settings;
// TODO this is wrong, we should only use the environment that is provided to createComponents
Expand Down Expand Up @@ -863,8 +862,7 @@ Collection<Object> createComponents(
authzService,
getSslService(),
securityContext.get(),
destructiveOperations,
clusterService
destructiveOperations
)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.DestructiveOperations;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.ssl.SslConfiguration;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.common.util.concurrent.RunOnce;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.gateway.GatewayService;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.SendRequestTransportException;
Expand Down Expand Up @@ -57,17 +55,14 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
private final Settings settings;
private final SecurityContext securityContext;

private volatile boolean isStateNotRecovered = true;

public SecurityServerTransportInterceptor(
Settings settings,
ThreadPool threadPool,
AuthenticationService authcService,
AuthorizationService authzService,
SSLService sslService,
SecurityContext securityContext,
DestructiveOperations destructiveOperations,
ClusterService clusterService
DestructiveOperations destructiveOperations
) {
this.settings = settings;
this.threadPool = threadPool;
Expand All @@ -76,7 +71,6 @@ public SecurityServerTransportInterceptor(
this.sslService = sslService;
this.securityContext = securityContext;
this.profileFilters = initializeProfileFilters(destructiveOperations);
clusterService.addListener(e -> isStateNotRecovered = e.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK));
}

@Override
Expand Down Expand Up @@ -176,16 +170,7 @@ public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
boolean forceExecution,
TransportRequestHandler<T> actualHandler
) {
return new ProfileSecuredRequestHandler<>(
logger,
action,
forceExecution,
executor,
actualHandler,
profileFilters,
settings,
threadPool
);
return new ProfileSecuredRequestHandler<>(logger, action, forceExecution, executor, actualHandler, profileFilters, threadPool);
}

private Map<String, ServerTransportFilter> initializeProfileFilters(DestructiveOperations destructiveOperations) {
Expand Down Expand Up @@ -232,7 +217,6 @@ public static class ProfileSecuredRequestHandler<T extends TransportRequest> imp
String executorName,
TransportRequestHandler<T> handler,
Map<String, ServerTransportFilter> profileFilters,
Settings settings,
ThreadPool threadPool
) {
this.logger = logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected XPackLicenseState getLicenseState() {
return thisVar.getLicenseState();
}
});
plugins.add(new Security(settings, configPath) {
plugins.add(new Security(settings) {
@Override
protected SSLService getSslService() {
return thisVar.getSslService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private Collection<Object> createComponentsUtil(Settings settings, SecurityExten
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(8), Version.CURRENT, Version.CURRENT);
licenseState = new TestUtils.UpdatableLicenseState(settings);
SSLService sslService = new SSLService(env);
security = new Security(settings, null, Arrays.asList(extensions)) {
security = new Security(settings, Arrays.asList(extensions)) {
@Override
protected XPackLicenseState getLicenseState() {
return licenseState;
Expand Down Expand Up @@ -658,7 +658,7 @@ public void testSecurityPluginInstallsRestHandlerWrapperEvenIfSecurityIsDisabled

try {
UsageService usageService = new UsageService();
Security security = new Security(settings, null);
Security security = new Security(settings);
assertTrue(security.getRestHandlerWrapper(threadPool.getThreadContext()) != null);

} finally {
Expand All @@ -680,7 +680,7 @@ public void testSecurityRestHandlerWrapperCanBeInstalled() throws IllegalAccessE

try {
UsageService usageService = new UsageService();
Security security = new Security(settings, null);
Security security = new Security(settings);

// Verify Security rest wrapper is about to be installed
// We will throw later if another wrapper is already installed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ public void testSendAsync() throws Exception {
new DestructiveOperations(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
),
clusterService
)
);
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener

Expand Down Expand Up @@ -144,8 +143,7 @@ public void testSendAsyncSwitchToSystem() throws Exception {
new DestructiveOperations(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
),
clusterService
)
);
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener

Expand Down Expand Up @@ -187,8 +185,7 @@ public void testSendWithoutUser() throws Exception {
new DestructiveOperations(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
),
clusterService
)
) {
@Override
void assertNoAuthentication(String action) {}
Expand Down Expand Up @@ -236,8 +233,7 @@ public void testSendToNewerVersionSetsCorrectVersion() throws Exception {
new DestructiveOperations(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
),
clusterService
)
);
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener

Expand Down Expand Up @@ -291,8 +287,7 @@ public void testSendToOlderVersionSetsCorrectVersion() throws Exception {
new DestructiveOperations(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
),
clusterService
)
);
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener

Expand Down Expand Up @@ -411,7 +406,6 @@ public void testProfileSecuredRequestHandlerDecrementsRefCountOnFailure() throws
profileName,
new ServerTransportFilter(null, null, threadContext, randomBoolean(), destructiveOperations, securityContext)
),
settings,
threadPool
);
final TransportChannel channel = mock(TransportChannel.class);
Expand Down