Skip to content

Commit 5a5d6e9

Browse files
rjernstrjernst
authored
Invert license security disabled helper method (#54043) (#54239)
Xpack license state contains a helper method to determine whether security is disabled due to license level defaults. Most code needs to know whether security is enabled, not disabled, but this method exists so that the security being explicitly disabled can be distinguished from licence level defaulting to disabled. However, in the case that security is explicitly disabled, the handlers in question are never registered, so security is implicitly not disabled explicitly, and thus we can share a single method to know whether licensing is enabled.
1 parent 611d98a commit 5a5d6e9

File tree

13 files changed

+46
-47
lines changed

13 files changed

+46
-47
lines changed

‎buildSrc/src/main/java/org/elasticsearch/gradle/test/DistroTestPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public void apply(Project project) {
153153
//
154154
// The shouldTestDocker property could be null, hence we use Boolean.TRUE.equals()
155155
boolean shouldExecute = distribution.getType() != Type.DOCKER
156+
156157
|| Boolean.TRUE.equals(vmProject.findProperty("shouldTestDocker"));
157158

158159
if (shouldExecute) {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ public boolean isFlattenedAllowed() {
586586
public boolean isVectorsAllowed() {
587587
return allowForAllLicenses();
588588
}
589-
589+
590590

591591
/**
592592
* Determine if Wildcard support should be enabled.
@@ -595,7 +595,7 @@ public boolean isVectorsAllowed() {
595595
*/
596596
public synchronized boolean isWildcardAllowed() {
597597
return status.active;
598-
}
598+
}
599599

600600
public boolean isOdbcAllowed() {
601601
return isAllowedByLicense(OperationMode.PLATINUM);
@@ -621,22 +621,11 @@ public boolean isSecurityAvailable() {
621621
}
622622

623623
/**
624-
* @return true if security has been disabled due it being the default setting for this license type.
625-
* The conditions necessary for this are:
626-
* <ul>
627-
* <li>A trial or basic license</li>
628-
* <li>xpack.security.enabled not specified as a setting</li>
629-
* </ul>
624+
* Returns whether security is enabled, taking into account the default enabled state
625+
* based on the current license level.
630626
*/
631-
public boolean isSecurityDisabledByLicenseDefaults() {
632-
return checkAgainstStatus(status -> {
633-
switch (status.mode) {
634-
case TRIAL:
635-
case BASIC:
636-
return isSecurityEnabled && isSecurityExplicitlyEnabled == false;
637-
}
638-
return false;
639-
});
627+
public boolean isSecurityEnabled() {
628+
return isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled);
640629
}
641630

642631
public static boolean isTransportTlsRequired(License license, Settings settings) {

‎x-pack/plugin/core/src/test/java/org/elasticsearch/license/XPackLicenseStateTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public void testSecurityBasicWithoutExplicitSecurityEnabled() {
110110
assertThat(licenseState.isApiKeyServiceAllowed(), is(false));
111111

112112
assertThat(licenseState.isSecurityAvailable(), is(true));
113-
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(true));
113+
assertThat(licenseState.isSecurityEnabled(), is(false));
114114
}
115115

116116
public void testSecurityBasicWithExplicitSecurityEnabled() {
@@ -128,7 +128,7 @@ public void testSecurityBasicWithExplicitSecurityEnabled() {
128128
assertThat(licenseState.isApiKeyServiceAllowed(), is(true));
129129

130130
assertThat(licenseState.isSecurityAvailable(), is(true));
131-
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(false));
131+
assertThat(licenseState.isSecurityEnabled(), is(true));
132132
}
133133

134134
public void testSecurityDefaultBasicExpired() {
@@ -254,7 +254,7 @@ public void testNewTrialDefaultsSecurityOff() {
254254
XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY);
255255
licenseState.update(TRIAL, true, VersionUtils.randomVersionBetween(random(), Version.V_6_3_0, Version.CURRENT));
256256

257-
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(true));
257+
assertThat(licenseState.isSecurityEnabled(), is(false));
258258
assertSecurityNotAllowed(licenseState);
259259
}
260260

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatureSet.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,7 @@ public boolean available() {
7676

7777
@Override
7878
public boolean enabled() {
79-
if (licenseState != null) {
80-
return XPackSettings.SECURITY_ENABLED.get(settings) &&
81-
licenseState.isSecurityDisabledByLicenseDefaults() == false;
82-
}
83-
return false;
79+
return licenseState != null && licenseState.isSecurityEnabled();
8480
}
8581

8682
@Override
@@ -101,13 +97,14 @@ public void usage(ActionListener<XPackFeatureSet.Usage> listener) {
10197
final AtomicReference<Map<String, Object>> rolesUsageRef = new AtomicReference<>();
10298
final AtomicReference<Map<String, Object>> roleMappingUsageRef = new AtomicReference<>();
10399
final AtomicReference<Map<String, Object>> realmsUsageRef = new AtomicReference<>();
100+
101+
final boolean enabled = licenseState.isSecurityEnabled();
104102
final CountDown countDown = new CountDown(3);
105103
final Runnable doCountDown = () -> {
106104
if (countDown.countDown()) {
107105
listener.onResponse(new SecurityFeatureSetUsage(available(), enabled(), realmsUsageRef.get(), rolesUsageRef.get(),
108106
roleMappingUsageRef.get(), sslUsage, auditUsage, ipFilterUsage, anonymousUsage, tokenServiceUsage,
109-
apiKeyServiceUsage, fips140Usage));
110-
}
107+
apiKeyServiceUsage, fips140Usage)); }
111108
};
112109

113110
final ActionListener<Map<String, Object>> rolesStoreUsageListener =
@@ -129,17 +126,17 @@ public void usage(ActionListener<XPackFeatureSet.Usage> listener) {
129126
doCountDown.run();
130127
}, listener::onFailure);
131128

132-
if (rolesStore == null) {
129+
if (rolesStore == null || enabled == false) {
133130
rolesStoreUsageListener.onResponse(Collections.emptyMap());
134131
} else {
135132
rolesStore.usageStats(rolesStoreUsageListener);
136133
}
137-
if (roleMappingStore == null) {
134+
if (roleMappingStore == null || enabled == false) {
138135
roleMappingStoreUsageListener.onResponse(Collections.emptyMap());
139136
} else {
140137
roleMappingStore.usageStats(roleMappingStoreUsageListener);
141138
}
142-
if (realms == null) {
139+
if (realms == null || enabled == false) {
143140
realmsUsageListener.onResponse(Collections.emptyMap());
144141
} else {
145142
realms.usageStats(realmsUsageListener);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public <Request extends ActionRequest, Response extends ActionResponse> void app
111111
listener.onFailure(e);
112112
}
113113
} else if (SECURITY_ACTION_MATCHER.test(action)) {
114-
if (licenseState.isSecurityDisabledByLicenseDefaults()) {
114+
if (licenseState.isSecurityEnabled() == false) {
115115
listener.onFailure(new ElasticsearchException("Security must be explicitly enabled when using a [" +
116116
licenseState.getOperationMode().description() + "] license. " +
117117
"Enable security by setting [xpack.security.enabled] to [true] in the elasticsearch.yml file " +

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ protected Exception checkFeatureAvailable(RestRequest request) {
7070
return new IllegalStateException("Security is not enabled but a security rest handler is registered");
7171
} else if (licenseState.isSecurityAvailable() == false) {
7272
return LicenseUtils.newComplianceException(XPackField.SECURITY);
73-
} else if (licenseState.isSecurityDisabledByLicenseDefaults()) {
73+
} else if (licenseState.isSecurityEnabled() == false) {
7474
return new ElasticsearchException("Security must be explicitly enabled when using a [" +
7575
licenseState.getOperationMode().description() + "] license. " +
7676
"Enable security by setting [xpack.security.enabled] to [true] in the elasticsearch.yml file " +

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityStatusChangeListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public SecurityStatusChangeListener(XPackLicenseState licenseState) {
3535
*/
3636
@Override
3737
public synchronized void licenseStateChanged() {
38-
final boolean newState = licenseState.isSecurityAvailable() && licenseState.isSecurityDisabledByLicenseDefaults() == false;
38+
final boolean newState = licenseState.isSecurityAvailable() && licenseState.isSecurityEnabled();
3939
// old state might be null (undefined) so do Object comparison
4040
if (Objects.equals(newState, securityEnabled) == false) {
4141
logger.info("Active license is now [{}]; Security is {}", licenseState.getOperationMode(), newState ? "enabled" : "disabled");

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,23 +79,27 @@ public void testAvailable() {
7979
public void testEnabled() {
8080
SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms,
8181
rolesStore, roleMappingStore, ipFilter);
82+
when(licenseState.isSecurityEnabled()).thenReturn(true);
8283
assertThat(featureSet.enabled(), is(true));
8384

84-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
85+
when(licenseState.isSecurityEnabled()).thenReturn(false);
8586
featureSet = new SecurityFeatureSet(settings, licenseState, realms,
8687
rolesStore, roleMappingStore, ipFilter);
8788
assertThat(featureSet.enabled(), is(false));
8889
}
8990

9091
public void testUsage() throws Exception {
9192
final boolean authcAuthzAvailable = randomBoolean();
93+
final boolean explicitlyDisabled = randomBoolean();
94+
final boolean enabled = explicitlyDisabled == false && randomBoolean();
9295
when(licenseState.isSecurityAvailable()).thenReturn(authcAuthzAvailable);
96+
when(licenseState.isSecurityEnabled()).thenReturn(enabled);
9397

9498
Settings.Builder settings = Settings.builder().put(this.settings);
9599

96-
boolean enabled = randomBoolean();
97-
settings.put(XPackSettings.SECURITY_ENABLED.getKey(), enabled);
98-
100+
if (explicitlyDisabled) {
101+
settings.put("xpack.security.enabled", "false");
102+
}
99103
final boolean httpSSLEnabled = randomBoolean();
100104
settings.put("xpack.security.http.ssl.enabled", httpSSLEnabled);
101105
final boolean transportSSLEnabled = randomBoolean();
@@ -224,8 +228,13 @@ public void testUsage() throws Exception {
224228
// FIPS 140
225229
assertThat(source.getValue("fips_140.enabled"), is(fips140Enabled));
226230
} else {
231+
if (explicitlyDisabled) {
232+
assertThat(source.getValue("ssl"), is(nullValue()));
233+
} else {
234+
assertThat(source.getValue("ssl.http.enabled"), is(httpSSLEnabled));
235+
assertThat(source.getValue("ssl.transport.enabled"), is(transportSSLEnabled));
236+
}
227237
assertThat(source.getValue("realms"), is(nullValue()));
228-
assertThat(source.getValue("ssl"), is(nullValue()));
229238
assertThat(source.getValue("token_service"), is(nullValue()));
230239
assertThat(source.getValue("api_key_service"), is(nullValue()));
231240
assertThat(source.getValue("audit"), is(nullValue()));
@@ -252,7 +261,7 @@ public void testUsage() throws Exception {
252261

253262
public void testUsageOnTrialLicenseWithSecurityDisabledByDefault() throws Exception {
254263
when(licenseState.isSecurityAvailable()).thenReturn(true);
255-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
264+
when(licenseState.isSecurityEnabled()).thenReturn(false);
256265

257266
Settings.Builder settings = Settings.builder().put(this.settings);
258267

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandlerTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727
public class SecurityBaseRestHandlerTests extends ESTestCase {
2828

2929
public void testSecurityBaseRestHandlerChecksLicenseState() throws Exception {
30-
final boolean securityDisabledByLicenseDefaults = randomBoolean();
30+
final boolean securityDefaultEnabled = randomBoolean();
3131
final AtomicBoolean consumerCalled = new AtomicBoolean(false);
3232
final XPackLicenseState licenseState = mock(XPackLicenseState.class);
3333
when(licenseState.isSecurityAvailable()).thenReturn(true);
34-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(securityDisabledByLicenseDefaults);
34+
when(licenseState.isSecurityEnabled()).thenReturn(securityDefaultEnabled);
3535
when(licenseState.getOperationMode()).thenReturn(
3636
randomFrom(License.OperationMode.BASIC, License.OperationMode.STANDARD, License.OperationMode.GOLD));
3737
SecurityBaseRestHandler handler = new SecurityBaseRestHandler(Settings.EMPTY, licenseState) {
@@ -56,15 +56,15 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien
5656
}
5757
};
5858
FakeRestRequest fakeRestRequest = new FakeRestRequest();
59-
FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), securityDisabledByLicenseDefaults ? 1 : 0);
59+
FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), securityDefaultEnabled ? 0 : 1);
6060
NodeClient client = mock(NodeClient.class);
6161

6262
assertFalse(consumerCalled.get());
6363
verifyZeroInteractions(licenseState);
6464
handler.handleRequest(fakeRestRequest, fakeRestChannel, client);
6565

6666
verify(licenseState).isSecurityAvailable();
67-
if (securityDisabledByLicenseDefaults == false) {
67+
if (securityDefaultEnabled) {
6868
assertTrue(consumerCalled.get());
6969
assertEquals(0, fakeRestChannel.responses().get());
7070
assertEquals(0, fakeRestChannel.errors().get());

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateApiKeyActionTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public void setUp() throws Exception {
5454
.build();
5555
threadPool = new ThreadPool(settings);
5656
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
57+
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
5758
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
5859
}
5960

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public void setUp() throws Exception {
5454
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build();
5555
threadPool = new ThreadPool(settings);
5656
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
57+
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
5758
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
5859
}
5960

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public void setUp() throws Exception {
5454
.build();
5555
threadPool = new ThreadPool(settings);
5656
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
57+
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
5758
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
5859
}
5960

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityStatusChangeListenerTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public void cleanup() {
4747
}
4848

4949
public void testSecurityEnabledToDisabled() {
50-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(false);
50+
when(licenseState.isSecurityEnabled()).thenReturn(true);
5151

5252
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.GOLD);
5353
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
@@ -66,7 +66,7 @@ public void testSecurityEnabledToDisabled() {
6666
"Active license is now [PLATINUM]; Security is enabled"
6767
));
6868

69-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
69+
when(licenseState.isSecurityEnabled()).thenReturn(false);
7070
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.BASIC);
7171
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
7272
"change to basic",
@@ -80,7 +80,7 @@ public void testSecurityEnabledToDisabled() {
8080
}
8181

8282
public void testSecurityDisabledToEnabled() {
83-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
83+
when(licenseState.isSecurityEnabled()).thenReturn(false);
8484

8585
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.TRIAL);
8686
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
@@ -99,7 +99,7 @@ public void testSecurityDisabledToEnabled() {
9999
"Active license is now [BASIC]; Security is disabled"
100100
));
101101

102-
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(false);
102+
when(licenseState.isSecurityEnabled()).thenReturn(true);
103103
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.PLATINUM);
104104
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
105105
"change to platinum",

0 commit comments

Comments
 (0)