Skip to content

Commit de3f122

Browse files
[8.2] Security plugin close realms (#87429) (#87443)
* Security plugin close realms (#87429) The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes #86286 * IOUtils rename around
1 parent 7f6f419 commit de3f122

File tree

5 files changed

+157
-4
lines changed

5 files changed

+157
-4
lines changed

docs/changelog/87429.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 87429
2+
summary: Security plugin close releasable realms
3+
area: Security
4+
type: bug
5+
issues:
6+
- 86286

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/SecurityPluginTests.java

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,36 @@
66
*/
77
package org.elasticsearch.xpack.security;
88

9+
import org.elasticsearch.action.ActionListener;
910
import org.elasticsearch.client.Request;
1011
import org.elasticsearch.client.RequestOptions;
1112
import org.elasticsearch.client.Response;
1213
import org.elasticsearch.client.ResponseException;
1314
import org.elasticsearch.common.settings.SecureString;
15+
import org.elasticsearch.common.settings.Setting;
16+
import org.elasticsearch.common.settings.Settings;
17+
import org.elasticsearch.common.util.concurrent.ThreadContext;
18+
import org.elasticsearch.plugins.Plugin;
1419
import org.elasticsearch.test.SecurityIntegTestCase;
1520
import org.elasticsearch.test.SecuritySettingsSource;
1621
import org.elasticsearch.test.SecuritySettingsSourceField;
22+
import org.elasticsearch.xpack.core.security.SecurityExtension;
23+
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
24+
import org.elasticsearch.xpack.core.security.authc.AuthenticationToken;
25+
import org.elasticsearch.xpack.core.security.authc.Realm;
26+
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
27+
import org.elasticsearch.xpack.core.security.authc.RealmSettings;
28+
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
29+
import org.elasticsearch.xpack.core.security.user.User;
1730

31+
import java.io.Closeable;
1832
import java.io.IOException;
33+
import java.nio.file.Path;
34+
import java.util.ArrayList;
35+
import java.util.Collection;
36+
import java.util.List;
37+
import java.util.Map;
38+
import java.util.concurrent.atomic.AtomicBoolean;
1939

2040
import static org.elasticsearch.rest.RestStatus.OK;
2141
import static org.elasticsearch.rest.RestStatus.UNAUTHORIZED;
@@ -29,6 +49,33 @@ protected boolean addMockHttpTransport() {
2949
return false; // enable http
3050
}
3151

52+
@Override
53+
protected Collection<Class<? extends Plugin>> nodePlugins() {
54+
final List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
55+
{
56+
// replace the security plugin with the security plugin that contains a dummy realm
57+
plugins.remove(LocalStateSecurity.class);
58+
plugins.add(SecurityPluginTests.LocalStateWithDummyRealmAuthorizationEngineExtension.class);
59+
}
60+
return List.copyOf(plugins);
61+
}
62+
63+
@Override
64+
protected Class<?> xpackPluginClass() {
65+
return SecurityPluginTests.LocalStateWithDummyRealmAuthorizationEngineExtension.class;
66+
}
67+
68+
private static final AtomicBoolean REALM_CLOSE_FLAG = new AtomicBoolean(false);
69+
private static final AtomicBoolean REALM_INIT_FLAG = new AtomicBoolean(false);
70+
71+
@Override
72+
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
73+
final Settings.Builder settingsBuilder = Settings.builder();
74+
settingsBuilder.put(super.nodeSettings(nodeOrdinal, otherSettings));
75+
settingsBuilder.put("xpack.security.authc.realms.dummy.dummy10.order", "10");
76+
return settingsBuilder.build();
77+
}
78+
3279
public void testThatPluginIsLoaded() throws IOException {
3380
try {
3481
logger.info("executing unauthorized request to /_xpack info");
@@ -53,4 +100,80 @@ public void testThatPluginIsLoaded() throws IOException {
53100
Response response = getRestClient().performRequest(request);
54101
assertThat(response.getStatusLine().getStatusCode(), is(OK.getStatus()));
55102
}
103+
104+
public void testThatPluginRealmIsLoadedAndClosed() throws IOException {
105+
REALM_CLOSE_FLAG.set(false);
106+
REALM_INIT_FLAG.set(false);
107+
String nodeName = internalCluster().startNode();
108+
assertThat(REALM_INIT_FLAG.get(), is(true));
109+
internalCluster().stopNode(nodeName);
110+
assertThat(REALM_CLOSE_FLAG.get(), is(true));
111+
}
112+
113+
public static class LocalStateWithDummyRealmAuthorizationEngineExtension extends LocalStateSecurity {
114+
115+
public LocalStateWithDummyRealmAuthorizationEngineExtension(Settings settings, Path configPath) throws Exception {
116+
super(settings, configPath);
117+
}
118+
119+
@Override
120+
protected List<SecurityExtension> securityExtensions() {
121+
return List.of(new DummyRealmAuthorizationEngineExtension());
122+
}
123+
124+
@Override
125+
public List<Setting<?>> getSettings() {
126+
ArrayList<Setting<?>> settings = new ArrayList<>();
127+
settings.addAll(super.getSettings());
128+
settings.addAll(RealmSettings.getStandardSettings(DummyRealm.TYPE));
129+
return settings;
130+
}
131+
}
132+
133+
public static class DummyRealmAuthorizationEngineExtension implements SecurityExtension {
134+
135+
DummyRealmAuthorizationEngineExtension() {}
136+
137+
@Override
138+
public Map<String, Realm.Factory> getRealms(SecurityComponents components) {
139+
return Map.of(DummyRealm.TYPE, DummyRealm::new);
140+
}
141+
}
142+
143+
public static class DummyRealm extends Realm implements Closeable {
144+
145+
public static final String TYPE = "dummy";
146+
147+
public DummyRealm(RealmConfig config) {
148+
super(config);
149+
REALM_INIT_FLAG.set(true);
150+
}
151+
152+
@Override
153+
public boolean supports(AuthenticationToken token) {
154+
return false;
155+
}
156+
157+
@Override
158+
public UsernamePasswordToken token(ThreadContext threadContext) {
159+
return null;
160+
}
161+
162+
@Override
163+
public void authenticate(AuthenticationToken authToken, ActionListener<AuthenticationResult<User>> listener) {
164+
listener.onResponse(AuthenticationResult.notHandled());
165+
}
166+
167+
@Override
168+
public void lookupUser(String username, ActionListener<User> listener) {
169+
// Lookup (run-as) is not supported in this realm
170+
listener.onResponse(null);
171+
}
172+
173+
@Override
174+
public void close() {
175+
REALM_CLOSE_FLAG.set(true);
176+
}
177+
}
178+
56179
}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111
import org.elasticsearch.action.ActionListener;
1212
import org.elasticsearch.common.Strings;
1313
import org.elasticsearch.common.collect.MapBuilder;
14+
import org.elasticsearch.common.component.AbstractLifecycleComponent;
1415
import org.elasticsearch.common.logging.DeprecationCategory;
1516
import org.elasticsearch.common.logging.DeprecationLogger;
1617
import org.elasticsearch.common.settings.Settings;
1718
import org.elasticsearch.common.util.Maps;
1819
import org.elasticsearch.common.util.concurrent.CountDown;
1920
import org.elasticsearch.common.util.concurrent.ThreadContext;
2021
import org.elasticsearch.core.Nullable;
22+
import org.elasticsearch.core.internal.io.IOUtils;
2123
import org.elasticsearch.env.Environment;
2224
import org.elasticsearch.license.LicensedFeature;
2325
import org.elasticsearch.license.XPackLicenseState;
@@ -32,6 +34,8 @@
3234
import org.elasticsearch.xpack.security.Security;
3335
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
3436

37+
import java.io.Closeable;
38+
import java.io.IOException;
3539
import java.util.ArrayList;
3640
import java.util.Collection;
3741
import java.util.Collections;
@@ -50,7 +54,7 @@
5054
/**
5155
* Serves as a realms registry (also responsible for ordering the realms appropriately)
5256
*/
53-
public class Realms implements Iterable<Realm> {
57+
public class Realms extends AbstractLifecycleComponent implements Iterable<Realm> {
5458

5559
private static final Logger logger = LogManager.getLogger(Realms.class);
5660
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());
@@ -339,6 +343,17 @@ public Map<String, Object> domainUsageStats() {
339343
}
340344
}
341345

346+
@Override
347+
protected void doStart() {}
348+
349+
@Override
350+
protected void doStop() {}
351+
352+
@Override
353+
protected void doClose() throws IOException {
354+
IOUtils.close(allConfiguredRealms.stream().filter(r -> r instanceof Closeable).map(r -> (Closeable) r).toList());
355+
}
356+
342357
private void maybeAddBasicRealms(List<Realm> realms, List<RealmConfig> realmConfigs) throws Exception {
343358
final Set<String> disabledBasicRealmTypes = findDisabledBasicRealmTypes(realmConfigs);
344359
final Set<String> realmTypes = realms.stream().map(Realm::type).collect(Collectors.toUnmodifiableSet());

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,17 @@ protected void doAssertXPackIsInstalled() {
226226
.map(p -> p.getClassname())
227227
.collect(Collectors.toList());
228228
assertThat(
229-
"plugin [" + LocalStateSecurity.class.getName() + "] not found in [" + pluginNames + "]",
229+
"plugin [" + xpackPluginClass().getName() + "] not found in [" + pluginNames + "]",
230230
pluginNames,
231-
hasItem(LocalStateSecurity.class.getName())
231+
hasItem(xpackPluginClass().getName())
232232
);
233233
}
234234
}
235235

236+
protected Class<?> xpackPluginClass() {
237+
return LocalStateSecurity.class;
238+
}
239+
236240
@Override
237241
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
238242
Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalStateSecurity.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.elasticsearch.xpack.core.action.XPackInfoFeatureAction;
2727
import org.elasticsearch.xpack.core.action.XPackUsageFeatureAction;
2828
import org.elasticsearch.xpack.core.action.XPackUsageResponse;
29+
import org.elasticsearch.xpack.core.security.SecurityExtension;
2930
import org.elasticsearch.xpack.core.ssl.SSLService;
3031
import org.elasticsearch.xpack.ilm.IndexLifecycle;
3132
import org.elasticsearch.xpack.monitoring.Monitoring;
@@ -97,7 +98,7 @@ protected XPackLicenseState getLicenseState() {
9798
return thisVar.getLicenseState();
9899
}
99100
});
100-
plugins.add(new Security(settings) {
101+
plugins.add(new Security(settings, thisVar.securityExtensions()) {
101102
@Override
102103
protected SSLService getSslService() {
103104
return thisVar.getSslService();
@@ -110,6 +111,10 @@ protected XPackLicenseState getLicenseState() {
110111
});
111112
}
112113

114+
protected List<SecurityExtension> securityExtensions() {
115+
return List.of();
116+
}
117+
113118
@Override
114119
protected Class<? extends TransportAction<XPackUsageRequest, XPackUsageResponse>> getUsageAction() {
115120
return SecurityTransportXPackUsageAction.class;

0 commit comments

Comments
 (0)