Security: use x-pack config files when present

x-pack/plugin/security/build.gradle

@@ -137,6 +137,13 @@ artifacts {
     archives jar
     testArtifacts testJar
 }
+
+processResources {
+    from('src/main/config') {
+        into 'config'
+    }
+}
+
 sourceSets.test.resources {
     srcDir '../core/src/test/resources'
 }

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

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.security;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.SetOnce;
 import org.elasticsearch.Version;
@@ -27,8 +28,8 @@
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.util.Providers;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.logging.LoggerMessageFormat;
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.regex.Regex;
@@ -221,7 +222,10 @@
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Clock;
 import java.util.ArrayList;
@@ -253,7 +257,8 @@
 public class Security extends Plugin implements ActionPlugin, IngestPlugin, NetworkPlugin, ClusterPlugin, DiscoveryPlugin, MapperPlugin,
         ExtensiblePlugin {
 
-    private static final Logger logger = Loggers.getLogger(Security.class);
+    private static final Logger LOGGER = LogManager.getLogger(Security.class);
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER);
 
     public static final String NAME4 = XPackField.SECURITY + "4";
     public static final Setting<Optional<String>> USER_SETTING =
@@ -535,7 +540,7 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
             extensionName = extension.toString();
         }
         if (failureHandler == null) {
-            logger.debug("Using default authentication failure handler");
+            LOGGER.debug("Using default authentication failure handler");
             final Map<String, List<String>> defaultFailureResponseHeaders = new HashMap<>();
             realms.asList().stream().forEach((realm) -> {
                 Map<String, List<String>> realmFailureHeaders = realm.getAuthenticationFailureHeaders();
@@ -556,7 +561,7 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
             }
             failureHandler = new DefaultAuthenticationFailureHandler(defaultFailureResponseHeaders);
         } else {
-            logger.debug("Using authentication failure handler from extension [" + extensionName + "]");
+            LOGGER.debug("Using authentication failure handler from extension [" + extensionName + "]");
         }
         return failureHandler;
     }
@@ -949,7 +954,7 @@ static void validateAutoCreateIndex(Settings settings) {
                 }
             }
 
-            logger.warn("the [action.auto_create_index] setting is configured to be restrictive [{}]. " +
+            LOGGER.warn("the [action.auto_create_index] setting is configured to be restrictive [{}]. " +
                     " for the next 6 months audit indices are allowed to be created, but please make sure" +
                     " that any future history indices after 6 months with the pattern " +
                     "[.security_audit_log*] are allowed to be created", value);
@@ -1039,7 +1044,7 @@ public UnaryOperator<Map<String, IndexTemplateMetaData>> getIndexTemplateMetaDat
                 templates.put(SECURITY_TEMPLATE_NAME, IndexTemplateMetaData.Builder.fromXContent(parser, SECURITY_TEMPLATE_NAME));
             } catch (IOException e) {
                 // TODO: should we handle this with a thrown exception?
-                logger.error("Error loading template [{}] as part of metadata upgrading", SECURITY_TEMPLATE_NAME);
+                LOGGER.error("Error loading template [{}] as part of metadata upgrading", SECURITY_TEMPLATE_NAME);
             }
 
             final byte[] auditTemplate = TemplateUtils.loadTemplate("/" + IndexAuditTrail.INDEX_TEMPLATE_NAME + ".json",
@@ -1049,12 +1054,12 @@ public UnaryOperator<Map<String, IndexTemplateMetaData>> getIndexTemplateMetaDat
                     .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, auditTemplate)) {
                 IndexTemplateMetaData auditMetadata = new IndexTemplateMetaData.Builder(
                         IndexTemplateMetaData.Builder.fromXContent(parser, IndexAuditTrail.INDEX_TEMPLATE_NAME))
-                        .settings(IndexAuditTrail.customAuditIndexSettings(settings, logger))
+                        .settings(IndexAuditTrail.customAuditIndexSettings(settings, LOGGER))
                         .build();
                 templates.put(IndexAuditTrail.INDEX_TEMPLATE_NAME, auditMetadata);
             } catch (IOException e) {
                 // TODO: should we handle this with a thrown exception?
-                logger.error("Error loading template [{}] as part of metadata upgrading", IndexAuditTrail.INDEX_TEMPLATE_NAME);
+                LOGGER.error("Error loading template [{}] as part of metadata upgrading", IndexAuditTrail.INDEX_TEMPLATE_NAME);
             }
 
             return templates;
@@ -1167,4 +1172,55 @@ public void accept(DiscoveryNode node, ClusterState state) {
     public void reloadSPI(ClassLoader loader) {
         securityExtensions.addAll(SecurityExtension.loadExtensions(loader));
     }
+
+    public static Path resolveConfigFile(Environment env, String name) {
+        final Path config = env.configFile().resolve(name);
+        final Path legacyConfig = env.configFile().resolve("x-pack").resolve(name);
+        // config and legacy config can be the same path if name is an absolute path
+        if (config.equals(legacyConfig) == false) {
+            final boolean configFileExists = Files.exists(config);
+            final boolean legacyConfigExists = Files.exists(legacyConfig);
+            if (configFileExists == false) {
+                if (legacyConfigExists) {
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] is in a deprecated location. Move from " +
+                        legacyConfig.toString() + " to " + config.toString());
+                    return legacyConfig;
+                }
+            } else if (legacyConfigExists) {
+                // there is a file in both locations
+                if (isDefaultFile(name, config)) {
+                    // use the legacy file as the new file is the default but warn user
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] exists in a deprecated location and non-deprecated " +
+                        "location. The file in the non-deprecated location is the default file. Using file found in the deprecated " +
+                        "location. Move " + legacyConfig.toString() + " to " + config.toString());
+                    return legacyConfig;
+                } else {
+                    // the regular file has been modified, but the old still exists, warn the user
+                    DEPRECATION_LOGGER.deprecated("Config file [" + name + "] exists in a deprecated location and non-deprecated " +
+                        "location. Using file found in the non-deprecated location [" + config.toString() + "]. Determine which file " +
+                        "should be kept and move it to " + config.toString() + ", then remove " + legacyConfig.toString());
+                }
+            }
+        }
+        return config;
+    }
+
+    static boolean isDefaultFile(String name, Path file) {
+        try (InputStream in = XPackPlugin.class.getResourceAsStream("/config/" + name)) {
+            if (in != null) {
+                try (InputStream fin = Files.newInputStream(file)) {
+                    int inValue = in.read();
+                    int finValue = fin.read();
+                    while (inValue != -1 && finValue != -1 && inValue == finValue) {
+                        inValue = in.read();
+                        finValue = fin.read();
+                    }
+                    return inValue == finValue;
+                }
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+        return false;
+    }
 }

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

@@ -16,7 +16,6 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
@@ -25,6 +24,7 @@
 import org.elasticsearch.xpack.core.security.support.Validation;
 import org.elasticsearch.xpack.core.security.support.Validation.Users;
 import org.elasticsearch.xpack.core.security.user.User;
+import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.support.SecurityFiles;
 
 import java.io.IOException;
@@ -93,7 +93,7 @@ public boolean userExists(String username) {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "users");
+        return Security.resolveConfigFile(env, "users");
     }
 
     /**

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

@@ -15,10 +15,10 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
 import org.elasticsearch.xpack.core.security.support.Validation;
+import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.support.SecurityFiles;
 
 import java.io.IOException;
@@ -80,7 +80,7 @@ public String[] roles(String username) {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "users_roles");
+        return Security.resolveConfigFile(env, "users_roles");
     }
 
     /**

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

@@ -30,10 +30,10 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.support.CachingRealm;
 import org.elasticsearch.xpack.core.security.authc.support.DnRoleMapperSettings;
+import org.elasticsearch.xpack.security.Security;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableMap;
@@ -80,7 +80,7 @@ synchronized void addListener(Runnable listener) {
 
     public static Path resolveFile(Settings settings, Environment env) {
         String location = DnRoleMapperSettings.ROLE_MAPPING_FILE_SETTING.get(settings);
-        return XPackPlugin.resolveConfigFile(env, location);
+        return Security.resolveConfigFile(env, location);
     }
 
     /**

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java

@@ -22,13 +22,13 @@
 import org.elasticsearch.watcher.FileChangesListener;
 import org.elasticsearch.watcher.FileWatcher;
 import org.elasticsearch.watcher.ResourceWatcherService;
-import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges;
 import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
 import org.elasticsearch.xpack.core.security.support.Validation;
+import org.elasticsearch.xpack.security.Security;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -130,7 +130,7 @@ Set<String> getAllRoleNames() {
     }
 
     public static Path resolveFile(Environment env) {
-        return XPackPlugin.resolveConfigFile(env, "roles.yml");
+        return Security.resolveConfigFile(env, "roles.yml");
     }
 
     public static Set<String> parseFileForRoleNames(Path path, Logger logger) {

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

@@ -49,6 +49,9 @@
 import org.junit.Before;
 
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -456,4 +459,63 @@ public void testGetFieldFilterSecurityEnabledDeprecatedSetting() throws Exceptio
                     "future release! See the breaking changes documentation for the next major version.");
         }
     }
+
+    /**
+     * Tests the default file comparison check. Note: this will fail when run in an IDE as the
+     * processing of resources isn't handled properly.
+     *
+     * TODO: can we add an assume for whether the test should run if it is an IDE context?
+     */
+    public void testIsDefaultFileCheck() throws Exception {
+        Path homeDir = createTempDir();
+        Path configDir = homeDir.resolve("config");
+        Path xPackConfigDir = configDir.resolve("x-pack");
+        Environment environment = new Environment(Settings.builder().put("path.home", homeDir).build(), configDir);
+
+        List<String> defaultFiles = Arrays.asList("roles.yml", "role_mapping.yml", "users", "users_roles");
+        Files.createDirectories(xPackConfigDir);
+
+        for (String defaultFileName : defaultFiles) {
+            logger.info("testing default file: {}", defaultFileName);
+            Path defaultFile = getDataPath("/config/" + defaultFileName);
+            final byte[] defaultBytes = Files.readAllBytes(defaultFile);
+            final Path defaultFileConfigPath = configDir.resolve(defaultFileName);
+            Path resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+
+            Files.write(defaultFileConfigPath, defaultBytes);
+            assertTrue(Security.isDefaultFile(defaultFileName, defaultFileConfigPath));
+
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+
+            // put a file in x-pack dir
+            final Path xPackFilePath = xPackConfigDir.resolve(defaultFileName);
+            Files.write(xPackFilePath, Collections.singletonList(randomAlphaOfLength(8)));
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(xPackFilePath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] exists in a deprecated location and non-deprecated location. The" +
+                " file in the non-deprecated location is the default file. Using file found in the deprecated location. Move " +
+                xPackFilePath + " to " + defaultFileConfigPath);
+
+            // modify file in new location
+            Files.write(defaultFileConfigPath, Collections.singletonList(randomAlphaOfLength(8)),
+                randomBoolean() ? StandardOpenOption.TRUNCATE_EXISTING : StandardOpenOption.APPEND);
+
+            assertFalse(Security.isDefaultFile(defaultFileName, defaultFileConfigPath));
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(defaultFileConfigPath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] exists in a deprecated location and non-deprecated location. " +
+                "Using file found in the non-deprecated location [" + defaultFileConfigPath + "]. Determine which file should be kept and" +
+                    " move it to " + defaultFileConfigPath + ", then remove " + xPackFilePath);
+
+            // remove default file
+            Files.delete(defaultFileConfigPath);
+            resolvedPath = Security.resolveConfigFile(environment, defaultFileName);
+            assertEquals(xPackFilePath, resolvedPath);
+            assertWarnings("Config file [" + defaultFileName + "] is in a deprecated location. Move from " +
+                xPackFilePath + " to " + defaultFileConfigPath);
+        }
+
+    }
 }