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.
This commit is contained in:
Ryan Ernst 2020-03-25 19:20:10 -07:00 committed by GitHub
parent 611d98a472
commit 5a5d6e9ef2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 46 additions and 47 deletions

View File

@ -153,6 +153,7 @@ public class DistroTestPlugin implements Plugin<Project> {
//
// The shouldTestDocker property could be null, hence we use Boolean.TRUE.equals()
boolean shouldExecute = distribution.getType() != Type.DOCKER
|| Boolean.TRUE.equals(vmProject.findProperty("shouldTestDocker"));
if (shouldExecute) {

View File

@ -621,22 +621,11 @@ public class XPackLicenseState {
}
/**
* @return true if security has been disabled due it being the default setting for this license type.
* The conditions necessary for this are:
* <ul>
* <li>A trial or basic license</li>
* <li>xpack.security.enabled not specified as a setting</li>
* </ul>
* Returns whether security is enabled, taking into account the default enabled state
* based on the current license level.
*/
public boolean isSecurityDisabledByLicenseDefaults() {
return checkAgainstStatus(status -> {
switch (status.mode) {
case TRIAL:
case BASIC:
return isSecurityEnabled && isSecurityExplicitlyEnabled == false;
}
return false;
});
public boolean isSecurityEnabled() {
return isSecurityEnabled(status.mode, isSecurityExplicitlyEnabled, isSecurityEnabled);
}
public static boolean isTransportTlsRequired(License license, Settings settings) {

View File

@ -110,7 +110,7 @@ public class XPackLicenseStateTests extends ESTestCase {
assertThat(licenseState.isApiKeyServiceAllowed(), is(false));
assertThat(licenseState.isSecurityAvailable(), is(true));
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(true));
assertThat(licenseState.isSecurityEnabled(), is(false));
}
public void testSecurityBasicWithExplicitSecurityEnabled() {
@ -128,7 +128,7 @@ public class XPackLicenseStateTests extends ESTestCase {
assertThat(licenseState.isApiKeyServiceAllowed(), is(true));
assertThat(licenseState.isSecurityAvailable(), is(true));
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(false));
assertThat(licenseState.isSecurityEnabled(), is(true));
}
public void testSecurityDefaultBasicExpired() {
@ -254,7 +254,7 @@ public class XPackLicenseStateTests extends ESTestCase {
XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY);
licenseState.update(TRIAL, true, VersionUtils.randomVersionBetween(random(), Version.V_6_3_0, Version.CURRENT));
assertThat(licenseState.isSecurityDisabledByLicenseDefaults(), is(true));
assertThat(licenseState.isSecurityEnabled(), is(false));
assertSecurityNotAllowed(licenseState);
}

View File

@ -76,11 +76,7 @@ public class SecurityFeatureSet implements XPackFeatureSet {
@Override
public boolean enabled() {
if (licenseState != null) {
return XPackSettings.SECURITY_ENABLED.get(settings) &&
licenseState.isSecurityDisabledByLicenseDefaults() == false;
}
return false;
return licenseState != null && licenseState.isSecurityEnabled();
}
@Override
@ -101,13 +97,14 @@ public class SecurityFeatureSet implements XPackFeatureSet {
final AtomicReference<Map<String, Object>> rolesUsageRef = new AtomicReference<>();
final AtomicReference<Map<String, Object>> roleMappingUsageRef = new AtomicReference<>();
final AtomicReference<Map<String, Object>> realmsUsageRef = new AtomicReference<>();
final boolean enabled = licenseState.isSecurityEnabled();
final CountDown countDown = new CountDown(3);
final Runnable doCountDown = () -> {
if (countDown.countDown()) {
listener.onResponse(new SecurityFeatureSetUsage(available(), enabled(), realmsUsageRef.get(), rolesUsageRef.get(),
roleMappingUsageRef.get(), sslUsage, auditUsage, ipFilterUsage, anonymousUsage, tokenServiceUsage,
apiKeyServiceUsage, fips140Usage));
}
apiKeyServiceUsage, fips140Usage)); }
};
final ActionListener<Map<String, Object>> rolesStoreUsageListener =
@ -129,17 +126,17 @@ public class SecurityFeatureSet implements XPackFeatureSet {
doCountDown.run();
}, listener::onFailure);
if (rolesStore == null) {
if (rolesStore == null || enabled == false) {
rolesStoreUsageListener.onResponse(Collections.emptyMap());
} else {
rolesStore.usageStats(rolesStoreUsageListener);
}
if (roleMappingStore == null) {
if (roleMappingStore == null || enabled == false) {
roleMappingStoreUsageListener.onResponse(Collections.emptyMap());
} else {
roleMappingStore.usageStats(roleMappingStoreUsageListener);
}
if (realms == null) {
if (realms == null || enabled == false) {
realmsUsageListener.onResponse(Collections.emptyMap());
} else {
realms.usageStats(realmsUsageListener);

View File

@ -111,7 +111,7 @@ public class SecurityActionFilter implements ActionFilter {
listener.onFailure(e);
}
} else if (SECURITY_ACTION_MATCHER.test(action)) {
if (licenseState.isSecurityDisabledByLicenseDefaults()) {
if (licenseState.isSecurityEnabled() == false) {
listener.onFailure(new ElasticsearchException("Security must be explicitly enabled when using a [" +
licenseState.getOperationMode().description() + "] license. " +
"Enable security by setting [xpack.security.enabled] to [true] in the elasticsearch.yml file " +

View File

@ -70,7 +70,7 @@ public abstract class SecurityBaseRestHandler extends BaseRestHandler {
return new IllegalStateException("Security is not enabled but a security rest handler is registered");
} else if (licenseState.isSecurityAvailable() == false) {
return LicenseUtils.newComplianceException(XPackField.SECURITY);
} else if (licenseState.isSecurityDisabledByLicenseDefaults()) {
} else if (licenseState.isSecurityEnabled() == false) {
return new ElasticsearchException("Security must be explicitly enabled when using a [" +
licenseState.getOperationMode().description() + "] license. " +
"Enable security by setting [xpack.security.enabled] to [true] in the elasticsearch.yml file " +

View File

@ -35,7 +35,7 @@ public class SecurityStatusChangeListener implements LicenseStateListener {
*/
@Override
public synchronized void licenseStateChanged() {
final boolean newState = licenseState.isSecurityAvailable() && licenseState.isSecurityDisabledByLicenseDefaults() == false;
final boolean newState = licenseState.isSecurityAvailable() && licenseState.isSecurityEnabled();
// old state might be null (undefined) so do Object comparison
if (Objects.equals(newState, securityEnabled) == false) {
logger.info("Active license is now [{}]; Security is {}", licenseState.getOperationMode(), newState ? "enabled" : "disabled");

View File

@ -79,9 +79,10 @@ public class SecurityFeatureSetTests extends ESTestCase {
public void testEnabled() {
SecurityFeatureSet featureSet = new SecurityFeatureSet(settings, licenseState, realms,
rolesStore, roleMappingStore, ipFilter);
when(licenseState.isSecurityEnabled()).thenReturn(true);
assertThat(featureSet.enabled(), is(true));
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
when(licenseState.isSecurityEnabled()).thenReturn(false);
featureSet = new SecurityFeatureSet(settings, licenseState, realms,
rolesStore, roleMappingStore, ipFilter);
assertThat(featureSet.enabled(), is(false));
@ -89,13 +90,16 @@ public class SecurityFeatureSetTests extends ESTestCase {
public void testUsage() throws Exception {
final boolean authcAuthzAvailable = randomBoolean();
final boolean explicitlyDisabled = randomBoolean();
final boolean enabled = explicitlyDisabled == false && randomBoolean();
when(licenseState.isSecurityAvailable()).thenReturn(authcAuthzAvailable);
when(licenseState.isSecurityEnabled()).thenReturn(enabled);
Settings.Builder settings = Settings.builder().put(this.settings);
boolean enabled = randomBoolean();
settings.put(XPackSettings.SECURITY_ENABLED.getKey(), enabled);
if (explicitlyDisabled) {
settings.put("xpack.security.enabled", "false");
}
final boolean httpSSLEnabled = randomBoolean();
settings.put("xpack.security.http.ssl.enabled", httpSSLEnabled);
final boolean transportSSLEnabled = randomBoolean();
@ -224,8 +228,13 @@ public class SecurityFeatureSetTests extends ESTestCase {
// FIPS 140
assertThat(source.getValue("fips_140.enabled"), is(fips140Enabled));
} else {
assertThat(source.getValue("realms"), is(nullValue()));
if (explicitlyDisabled) {
assertThat(source.getValue("ssl"), is(nullValue()));
} else {
assertThat(source.getValue("ssl.http.enabled"), is(httpSSLEnabled));
assertThat(source.getValue("ssl.transport.enabled"), is(transportSSLEnabled));
}
assertThat(source.getValue("realms"), is(nullValue()));
assertThat(source.getValue("token_service"), is(nullValue()));
assertThat(source.getValue("api_key_service"), is(nullValue()));
assertThat(source.getValue("audit"), is(nullValue()));
@ -252,7 +261,7 @@ public class SecurityFeatureSetTests extends ESTestCase {
public void testUsageOnTrialLicenseWithSecurityDisabledByDefault() throws Exception {
when(licenseState.isSecurityAvailable()).thenReturn(true);
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
when(licenseState.isSecurityEnabled()).thenReturn(false);
Settings.Builder settings = Settings.builder().put(this.settings);

View File

@ -27,11 +27,11 @@ import static org.mockito.Mockito.when;
public class SecurityBaseRestHandlerTests extends ESTestCase {
public void testSecurityBaseRestHandlerChecksLicenseState() throws Exception {
final boolean securityDisabledByLicenseDefaults = randomBoolean();
final boolean securityDefaultEnabled = randomBoolean();
final AtomicBoolean consumerCalled = new AtomicBoolean(false);
final XPackLicenseState licenseState = mock(XPackLicenseState.class);
when(licenseState.isSecurityAvailable()).thenReturn(true);
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(securityDisabledByLicenseDefaults);
when(licenseState.isSecurityEnabled()).thenReturn(securityDefaultEnabled);
when(licenseState.getOperationMode()).thenReturn(
randomFrom(License.OperationMode.BASIC, License.OperationMode.STANDARD, License.OperationMode.GOLD));
SecurityBaseRestHandler handler = new SecurityBaseRestHandler(Settings.EMPTY, licenseState) {
@ -56,7 +56,7 @@ public class SecurityBaseRestHandlerTests extends ESTestCase {
}
};
FakeRestRequest fakeRestRequest = new FakeRestRequest();
FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), securityDisabledByLicenseDefaults ? 1 : 0);
FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), securityDefaultEnabled ? 0 : 1);
NodeClient client = mock(NodeClient.class);
assertFalse(consumerCalled.get());
@ -64,7 +64,7 @@ public class SecurityBaseRestHandlerTests extends ESTestCase {
handler.handleRequest(fakeRestRequest, fakeRestChannel, client);
verify(licenseState).isSecurityAvailable();
if (securityDisabledByLicenseDefaults == false) {
if (securityDefaultEnabled) {
assertTrue(consumerCalled.get());
assertEquals(0, fakeRestChannel.responses().get());
assertEquals(0, fakeRestChannel.errors().get());

View File

@ -54,6 +54,7 @@ public class RestCreateApiKeyActionTests extends ESTestCase {
.build();
threadPool = new ThreadPool(settings);
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
}

View File

@ -54,6 +54,7 @@ public class RestGetApiKeyActionTests extends ESTestCase {
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build();
threadPool = new ThreadPool(settings);
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
}

View File

@ -54,6 +54,7 @@ public class RestInvalidateApiKeyActionTests extends ESTestCase {
.build();
threadPool = new ThreadPool(settings);
when(mockLicenseState.isSecurityAvailable()).thenReturn(true);
when(mockLicenseState.isSecurityEnabled()).thenReturn(true);
when(mockLicenseState.isApiKeyServiceAllowed()).thenReturn(true);
}

View File

@ -47,7 +47,7 @@ public class SecurityStatusChangeListenerTests extends ESTestCase {
}
public void testSecurityEnabledToDisabled() {
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(false);
when(licenseState.isSecurityEnabled()).thenReturn(true);
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.GOLD);
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
@ -66,7 +66,7 @@ public class SecurityStatusChangeListenerTests extends ESTestCase {
"Active license is now [PLATINUM]; Security is enabled"
));
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
when(licenseState.isSecurityEnabled()).thenReturn(false);
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.BASIC);
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
"change to basic",
@ -80,7 +80,7 @@ public class SecurityStatusChangeListenerTests extends ESTestCase {
}
public void testSecurityDisabledToEnabled() {
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(true);
when(licenseState.isSecurityEnabled()).thenReturn(false);
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.TRIAL);
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
@ -99,7 +99,7 @@ public class SecurityStatusChangeListenerTests extends ESTestCase {
"Active license is now [BASIC]; Security is disabled"
));
when(licenseState.isSecurityDisabledByLicenseDefaults()).thenReturn(false);
when(licenseState.isSecurityEnabled()).thenReturn(true);
when(licenseState.getOperationMode()).thenReturn(License.OperationMode.PLATINUM);
logAppender.addExpectation(new MockLogAppender.SeenEventExpectation(
"change to platinum",