Skip to content

Commit

Permalink
Add warnings for invalid realm order config (#51195) (#51515)
Browse files Browse the repository at this point in the history
The changes are to help users prepare for migration to next major
release (v8.0.0) regarding to the break change of realm order config.

Warnings are added for when:
* A realm does not have an order config
* Multiple realms have the same order config

The warning messages are added to both deprecation API and loggings.
The main reasons for doing this are: 1) there is currently no automatic relay
between the two; 2) deprecation API is under basic and we need logging
for OSS.
  • Loading branch information
ywangd authored Jan 31, 2020
1 parent 11e86b1 commit 77b00fc
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 4 deletions.
1 change: 1 addition & 0 deletions docs/reference/migration/index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ For information about how to upgrade your cluster, see <<setup-upgrade>>.

--

include::migrate_7_7.asciidoc[]
include::migrate_7_6.asciidoc[]
include::migrate_7_5.asciidoc[]
include::migrate_7_4.asciidoc[]
Expand Down
19 changes: 19 additions & 0 deletions docs/reference/migration/migrate_7_7.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,22 @@ loggers in the `org.elasticsearch.action.*` hierarchy emitted log messages at
log noise. From 7.7 onwards the default log level for logger in this hierarchy
is now `INFO`, in line with most other loggers. If needed, you can recover the
pre-7.7 default behaviour by adjusting your <<logging>>.

[discrete]
[[breaking_77_settings_changes]]
=== Settings changes

[discrete]
[[deprecate-missing-realm-order]]
==== Authentication realm `order` will be a required config in version 8.0.0.

The `order` config will be required in version 8.0.0 for authentication realm
configuration of any type. If the `order` config is missing for a realm, the node
will fail to start.

[discrete]
[[deprecate-duplicated-realm-orders]]
==== Authentication realm `order` uniqueness will be enforced in version 8.0.0.

The `order` config of authentication realms must be unique in version 8.0.0.
If you configure more than one realm of any type with the same order, the node will fail to start.
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@
public class RealmSettings {

public static final String PREFIX = "xpack.security.authc.realms.";
public static final String ORDER_SETTING_KEY = "order";

public static final Function<String, Setting.AffixSetting<Boolean>> ENABLED_SETTING = affixSetting("enabled",
key -> Setting.boolSetting(key, true, Setting.Property.NodeScope));
public static final Function<String, Setting.AffixSetting<Integer>> ORDER_SETTING = affixSetting("order",
public static final Function<String, Setting.AffixSetting<Integer>> ORDER_SETTING = affixSetting(ORDER_SETTING_KEY,
key -> Setting.intSetting(key, Integer.MAX_VALUE, Setting.Property.NodeScope));

public static String realmSettingPrefix(String type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ private DeprecationChecks() {
static List<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> NODE_SETTINGS_CHECKS =
Collections.unmodifiableList(Arrays.asList(
NodeDeprecationChecks::checkPidfile,
NodeDeprecationChecks::checkProcessors
NodeDeprecationChecks::checkProcessors,
NodeDeprecationChecks::checkMissingRealmOrders,
NodeDeprecationChecks::checkUniqueRealmOrders
));

static List<Function<IndexMetaData, DeprecationIssue>> INDEX_SETTINGS_CHECKS =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.env.Environment;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;

import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

class NodeDeprecationChecks {

Expand All @@ -35,6 +40,62 @@ static DeprecationIssue checkProcessors(final Settings settings , final PluginsA
"https://www.elastic.co/guide/en/elasticsearch/reference/7.4/breaking-changes-7.4.html#deprecate-processors");
}

static DeprecationIssue checkMissingRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
final Set<String> orderNotConfiguredRealms = RealmSettings.getRealmSettings(settings).entrySet()
.stream()
.filter(e -> false == e.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY))
.map(e -> RealmSettings.realmSettingPrefix(e.getKey()) + RealmSettings.ORDER_SETTING_KEY)
.collect(Collectors.toSet());

if (orderNotConfiguredRealms.isEmpty()) {
return null;
}

final String details = String.format(
Locale.ROOT,
"Found realms without order config: [%s]. In next major release, node will fail to start with missing realm order.",
String.join("; ", orderNotConfiguredRealms));
return new DeprecationIssue(
DeprecationIssue.Level.CRITICAL,
"Realm order will be required in next major release.",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-missing-realm-order",
details
);
}

static DeprecationIssue checkUniqueRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
final Map<String, List<String>> orderToRealmSettings =
RealmSettings.getRealmSettings(settings).entrySet()
.stream()
.filter(e -> e.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY))
.collect(Collectors.groupingBy(
e -> e.getValue().get(RealmSettings.ORDER_SETTING_KEY),
Collectors.mapping(e -> RealmSettings.realmSettingPrefix(e.getKey()) + RealmSettings.ORDER_SETTING_KEY,
Collectors.toList())));

Set<String> duplicateOrders = orderToRealmSettings.entrySet().stream()
.filter(entry -> entry.getValue().size() > 1)
.map(entry -> entry.getKey() + ": " + entry.getValue())
.collect(Collectors.toSet());

if (duplicateOrders.isEmpty()) {
return null;
}

final String details = String.format(
Locale.ROOT,
"Found multiple realms configured with the same order: [%s]. " +
"In next major release, node will fail to start with duplicated realm order.",
String.join("; ", duplicateOrders));

return new DeprecationIssue(
DeprecationIssue.Level.CRITICAL,
"Realm orders must be unique in next major release.",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-duplicated-realm-orders",
details
);
}

private static DeprecationIssue checkDeprecatedSetting(
final Settings settings,
final PluginsAndModules pluginsAndModules,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;

import java.util.Collections;
import java.util.List;
import java.util.Locale;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;

public class NodeDeprecationChecksTests extends ESTestCase {

Expand Down Expand Up @@ -60,4 +66,81 @@ public void testCheckProcessors() {
assertSettingDeprecationsAndWarnings(new Setting<?>[]{EsExecutors.PROCESSORS_SETTING});
}

public void testCheckMissingRealmOrders() {
final RealmConfig.RealmIdentifier invalidRealm =
new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
final RealmConfig.RealmIdentifier validRealm =
new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
final Settings settings =
Settings.builder()
.put("xpack.security.authc.realms." + invalidRealm.getType() + "." + invalidRealm.getName() + ".enabled", "true")
.put("xpack.security.authc.realms." + validRealm.getType() + "." + validRealm.getName() + ".order", randomInt())
.build();

final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
final List<DeprecationIssue> deprecationIssues =
DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));

assertEquals(1, deprecationIssues.size());
assertEquals(new DeprecationIssue(
DeprecationIssue.Level.CRITICAL,
"Realm order will be required in next major release.",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-missing-realm-order",
String.format(
Locale.ROOT,
"Found realms without order config: [%s]. In next major release, node will fail to start with missing realm order.",
RealmSettings.realmSettingPrefix(invalidRealm) + RealmSettings.ORDER_SETTING_KEY
)
), deprecationIssues.get(0));
}

public void testCheckUniqueRealmOrders() {
final int order = randomInt(9999);

final RealmConfig.RealmIdentifier invalidRealm1 =
new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
final RealmConfig.RealmIdentifier invalidRealm2 =
new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
final RealmConfig.RealmIdentifier validRealm =
new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
final Settings settings = Settings.builder()
.put("xpack.security.authc.realms."
+ invalidRealm1.getType() + "." + invalidRealm1.getName() + ".order", order)
.put("xpack.security.authc.realms."
+ invalidRealm2.getType() + "." + invalidRealm2.getName() + ".order", order)
.put("xpack.security.authc.realms."
+ validRealm.getType() + "." + validRealm.getName() + ".order", order + 1)
.build();

final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
final List<DeprecationIssue> deprecationIssues =
DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));

assertEquals(1, deprecationIssues.size());
assertEquals(DeprecationIssue.Level.CRITICAL, deprecationIssues.get(0).getLevel());
assertEquals(
"https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-duplicated-realm-orders",
deprecationIssues.get(0).getUrl());
assertEquals("Realm orders must be unique in next major release.", deprecationIssues.get(0).getMessage());
assertThat(deprecationIssues.get(0).getDetails(), startsWith("Found multiple realms configured with the same order:"));
assertThat(deprecationIssues.get(0).getDetails(), containsString(invalidRealm1.getType() + "." + invalidRealm1.getName()));
assertThat(deprecationIssues.get(0).getDetails(), containsString(invalidRealm2.getType() + "." + invalidRealm2.getName()));
assertThat(deprecationIssues.get(0).getDetails(), not(containsString(validRealm.getType() + "." + validRealm.getName())));
}

public void testCorrectRealmOrders() {
final int order = randomInt(9999);
final Settings settings = Settings.builder()
.put("xpack.security.authc.realms."
+ randomAlphaOfLengthBetween(4, 12) + "." + randomAlphaOfLengthBetween(4, 12) + ".order", order)
.put("xpack.security.authc.realms."
+ randomAlphaOfLengthBetween(4, 12) + "." + randomAlphaOfLengthBetween(4, 12) + ".order", order + 1)
.build();

final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
final List<DeprecationIssue> deprecationIssues =
DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));

assertEquals(0, deprecationIssues.size());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.CountDown;
import org.elasticsearch.common.util.concurrent.ThreadContext;
Expand All @@ -34,6 +35,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -45,6 +47,7 @@
public class Realms implements Iterable<Realm> {

private static final Logger logger = LogManager.getLogger(Realms.class);
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);

private final Settings settings;
private final Environment env;
Expand Down Expand Up @@ -186,7 +189,16 @@ protected List<Realm> initRealms() throws Exception {
List<Realm> realms = new ArrayList<>();
List<String> kerberosRealmNames = new ArrayList<>();
Map<String, Set<String>> nameToRealmIdentifier = new HashMap<>();
for (RealmConfig.RealmIdentifier identifier: realmsSettings.keySet()) {
Set<String> missingOrderRealmSettingKeys = new TreeSet<>();
Map<String, Set<String>> orderToRealmOrderSettingKeys = new HashMap<>();
for (final Map.Entry<RealmConfig.RealmIdentifier, Settings> entry: realmsSettings.entrySet()) {
final RealmConfig.RealmIdentifier identifier = entry.getKey();
if (false == entry.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY)) {
missingOrderRealmSettingKeys.add(RealmSettings.getFullSettingKey(identifier, RealmSettings.ORDER_SETTING));
} else {
orderToRealmOrderSettingKeys.computeIfAbsent(entry.getValue().get(RealmSettings.ORDER_SETTING_KEY), k -> new TreeSet<>())
.add(RealmSettings.getFullSettingKey(identifier, RealmSettings.ORDER_SETTING));
}
Realm.Factory factory = factories.get(identifier.getType());
if (factory == null) {
throw new IllegalArgumentException("unknown realm type [" + identifier.getType() + "] for realm [" + identifier + "]");
Expand Down Expand Up @@ -236,6 +248,9 @@ protected List<Realm> initRealms() throws Exception {
if (Strings.hasText(duplicateRealms)) {
throw new IllegalArgumentException("Found multiple realms configured with the same name: " + duplicateRealms + "");
}

logDeprecationIfFound(missingOrderRealmSettingKeys, orderToRealmOrderSettingKeys);

return realms;
}

Expand Down Expand Up @@ -354,4 +369,24 @@ public static boolean isRealmTypeAvailable(AllowedRealmType enabledRealmType, St
}
}

private void logDeprecationIfFound(Set<String> missingOrderRealmSettingKeys, Map<String, Set<String>> orderToRealmOrderSettingKeys) {
if (missingOrderRealmSettingKeys.size() > 0) {
deprecationLogger.deprecated("Found realms without order config: [{}]. " +
"In next major release, node will fail to start with missing realm order.",
String.join("; ", missingOrderRealmSettingKeys)
);
}
final List<String> duplicatedRealmOrderSettingKeys = orderToRealmOrderSettingKeys.entrySet()
.stream()
.filter(e -> e.getValue().size() > 1)
.map(e -> e.getKey() + ": " + String.join(",", e.getValue()))
.sorted()
.collect(Collectors.toList());
if (false == duplicatedRealmOrderSettingKeys.isEmpty()) {
deprecationLogger.deprecated("Found multiple realms configured with the same order: [{}]. " +
"In next major release, node will fail to start with duplicated realm order.",
String.join("; ", duplicatedRealmOrderSettingKeys));
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.TreeMap;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -65,7 +66,7 @@ public void init() throws Exception {
factories.put(FileRealmSettings.TYPE, config -> new DummyRealm(FileRealmSettings.TYPE, config));
factories.put(NativeRealmSettings.TYPE, config -> new DummyRealm(NativeRealmSettings.TYPE, config));
factories.put(KerberosRealmSettings.TYPE, config -> new DummyRealm(KerberosRealmSettings.TYPE, config));
randomRealmTypesCount = randomIntBetween(1, 5);
randomRealmTypesCount = randomIntBetween(2, 5);
for (int i = 0; i < randomRealmTypesCount; i++) {
String name = "type_" + i;
factories.put(name, config -> new DummyRealm(name, config));
Expand Down Expand Up @@ -154,6 +155,13 @@ public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception

assertThat(realms.getUnlicensedRealms(), empty());
assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));

assertWarnings("Found multiple realms configured with the same order: [1: "
+ nameToRealmId.entrySet().stream()
.map(e -> "xpack.security.authc.realms.type_" + e.getValue() + "." + e.getKey() + ".order")
.sorted().collect(Collectors.joining(","))
+ "]. "
+ "In next major release, node will fail to start with duplicated realm order.");
}

public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception {
Expand Down Expand Up @@ -614,6 +622,20 @@ public void testInitRealmsFailsForMultipleKerberosRealms() throws IOException {
"multiple realms [realm_1, realm_2] configured of type [kerberos], [kerberos] can only have one such realm configured")));
}

public void testWarningForMissingRealmOrder() throws Exception {
final int realmTypeId = randomIntBetween(0, randomRealmTypesCount - 1);
final String realmName = randomAlphaOfLengthBetween(4, 12);
final Settings settings = Settings.builder()
.put("path.home", createTempDir())
.put("xpack.security.authc.realms.type_" + realmTypeId + ".realm_" + realmName + ".enabled", true)
.build();

new Realms(settings, TestEnvironment.newEnvironment(settings), factories, licenseState, threadContext, reservedRealm);
assertWarnings("Found realms without order config: [xpack.security.authc.realms.type_"
+ realmTypeId + ".realm_" + realmName + ".order]. "
+ "In next major release, node will fail to start with missing realm order.");
}

static class DummyRealm extends Realm {

DummyRealm(String type, RealmConfig config) {
Expand Down

0 comments on commit 77b00fc

Please sign in to comment.