From d1ee1f41e6e718f59e604af4fd83480fab8e6f9b Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 3 Sep 2021 16:05:24 -0500 Subject: [PATCH] Ading a check for saml nameid_format setting --- .../xpack/deprecation/DeprecationChecks.java | 1 + .../deprecation/NodeDeprecationChecks.java | 37 +++++++++- .../NodeDeprecationChecksTests.java | 67 ++++++++++++++++++- 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java index d9da43df6bd9d..26d8dc8d94d5e 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java @@ -117,6 +117,7 @@ private DeprecationChecks() { NodeDeprecationChecks::checkAcceptRolesCacheMaxSizeSetting, NodeDeprecationChecks::checkRolesCacheTTLSizeSetting, NodeDeprecationChecks::checkMaxLocalStorageNodesSetting, + NodeDeprecationChecks::checkSamlNameIdFormatSetting, NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting ) ).collect(Collectors.toList()); diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java index 569e6db14db20..3b2607ac706bc 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java @@ -20,8 +20,8 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.ssl.SslConfigurationKeys; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.TimeValue; @@ -36,9 +36,9 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.transport.RemoteClusterService; -import org.elasticsearch.xpack.core.DataTier; import org.elasticsearch.transport.SniffConnectionStrategy; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.DataTier; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.SecurityField; import org.elasticsearch.xpack.core.security.authc.RealmConfig; @@ -63,6 +63,7 @@ import static org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider.CLUSTER_ROUTING_INCLUDE_SETTING; import static org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider.CLUSTER_ROUTING_REQUIRE_SETTING; import static org.elasticsearch.xpack.core.security.authc.RealmSettings.RESERVED_REALM_NAME_PREFIX; +import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.PRINCIPAL_ATTRIBUTE; class NodeDeprecationChecks { @@ -970,4 +971,36 @@ static DeprecationIssue checkMaxLocalStorageNodesSetting(final Settings settings DeprecationIssue.Level.CRITICAL ); } + + static DeprecationIssue checkSamlNameIdFormatSetting(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + final String principalKeySuffix = ".attributes.principal"; + List detailsList = + PRINCIPAL_ATTRIBUTE.getAttribute().getAllConcreteSettings(settings).sorted(Comparator.comparing(Setting::getKey)) + .map(concreteSamlPrincipalSetting -> { + String concreteSamlPrincipalSettingKey = concreteSamlPrincipalSetting.getKey(); + int principalKeySuffixIndex = concreteSamlPrincipalSettingKey.indexOf(principalKeySuffix); + if (principalKeySuffixIndex > 0) { + String realm = concreteSamlPrincipalSettingKey.substring(0, principalKeySuffixIndex); + String concreteNameIdFormatSettingKey = realm + ".nameid_format"; + if (settings.get(concreteNameIdFormatSettingKey) == null) { + return String.format(Locale.ROOT, "no value for [%s] set in realm [%s]", + concreteNameIdFormatSettingKey, realm); + } + } + return null; + }) + .filter(detail -> detail != null).collect(Collectors.toList()); + if (detailsList.isEmpty()) { + return null; + } else { + String message = "if nameid_format is not explicitly set, the previous default of " + + "'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' is no longer used"; + String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/saml-guide.html"; + String details = detailsList.stream().collect(Collectors.joining(",")); + return new DeprecationIssue(DeprecationIssue.Level.WARNING, message, url, details, false, null); + } + } } diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java index 1b2520309c826..65f13ea3cb6bd 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java @@ -1243,8 +1243,6 @@ public void testCheckDelayClusterStateRecoverySettings() { } public void testCheckFixedAutoQueueSizeThreadpool() { - String settingKey = "thread_pool.search.min_queue_size"; - String settingValue = ""; Settings settings = Settings.builder() .put("thread_pool.search.min_queue_size", randomIntBetween(30, 100)) .put("thread_pool.search.max_queue_size", randomIntBetween(1, 25)) @@ -1404,4 +1402,69 @@ public void testCheckMaxLocalStorageNodesSetting() { String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_node_changes"; checkSimpleSetting(settingKey, settingValue, url, NodeDeprecationChecks::checkMaxLocalStorageNodesSetting); } + + public void testCheckSamlNameIdFormatSetting() { + Settings settings = Settings.builder() + .put("xpack.security.authc.realms.saml.saml1.attributes.principal", randomIntBetween(30, 100)) + .put("xpack.security.authc.realms.saml.saml1.nameid_format", randomIntBetween(1, 25)) + .build(); + final ClusterState clusterState = ClusterState.EMPTY_STATE; + final XPackLicenseState licenseState = mock(XPackLicenseState.class); + when(licenseState.getOperationMode()) + .thenReturn(randomValueOtherThanMany((m -> m.equals(License.OperationMode.BASIC) || m.equals(License.OperationMode.TRIAL)), + () -> randomFrom(License.OperationMode.values()))); + assertThat( + NodeDeprecationChecks.checkSamlNameIdFormatSetting(settings, null, clusterState, licenseState), + equalTo(null) + ); + + settings = Settings.builder() + .put("xpack.security.authc.realms.saml.saml1.attributes.principal", randomIntBetween(30, 100)) + .build(); + DeprecationIssue expectedIssue = new DeprecationIssue(DeprecationIssue.Level.WARNING, + "if nameid_format is not explicitly set, the previous default of 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' is no " + + "longer used", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/saml-guide.html", + "no value for [xpack.security.authc.realms.saml.saml1.nameid_format] set in realm [xpack.security.authc.realms.saml.saml1]", + false, null + ); + assertThat( + NodeDeprecationChecks.checkSamlNameIdFormatSetting(settings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + + settings = Settings.builder() + .put("xpack.security.authc.realms.saml.saml1.attributes.principal", randomIntBetween(30, 100)) + .put("xpack.security.authc.realms.saml.saml2.attributes.principal", randomIntBetween(30, 100)) + .put("xpack.security.authc.realms.saml.saml2.nameid_format", randomIntBetween(1, 25)) + .build(); + expectedIssue = new DeprecationIssue(DeprecationIssue.Level.WARNING, + "if nameid_format is not explicitly set, the previous default of 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' is no " + + "longer used", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/saml-guide.html", + "no value for [xpack.security.authc.realms.saml.saml1.nameid_format] set in realm [xpack.security.authc.realms.saml.saml1]", + false, null + ); + assertThat( + NodeDeprecationChecks.checkSamlNameIdFormatSetting(settings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + + settings = Settings.builder() + .put("xpack.security.authc.realms.saml.saml1.attributes.principal", randomIntBetween(30, 100)) + .put("xpack.security.authc.realms.saml.saml2.attributes.principal", randomIntBetween(30, 100)) + .build(); + expectedIssue = new DeprecationIssue(DeprecationIssue.Level.WARNING, + "if nameid_format is not explicitly set, the previous default of 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' is no " + + "longer used", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/saml-guide.html", + "no value for [xpack.security.authc.realms.saml.saml1.nameid_format] set in realm [xpack.security.authc.realms.saml.saml1]," + + "no value for [xpack.security.authc.realms.saml.saml2.nameid_format] set in realm [xpack.security.authc.realms.saml.saml2]", + false, null + ); + assertThat( + NodeDeprecationChecks.checkSamlNameIdFormatSetting(settings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + } }