diff --git a/docker/README.md b/docker/README.md index 869b5d819e..bf8d28c95e 100644 --- a/docker/README.md +++ b/docker/README.md @@ -59,6 +59,18 @@ compatible with the existing authentication protocol and the chart's documented password constraints. When the signing-key environment variable is omitted, the image reuses the configured key instead of rotating it. +Instead of `auth.admin_pa`, a REST configuration you mount yourself can set +`auth.admin_pa_file` to the path of a mounted secret. The Server reads that file as +raw UTF-8 and drops a leading BOM and one trailing line break, so the password +needs no properties escaping. The path must lead to a regular file, though +symlinks such as the ones Kubernetes uses for mounted Secrets are followed. The +password may be at most 72 bytes in UTF-8, because the password hash ignores +anything after that. When set, it overrides `auth.admin_pa` and the +init-store password prompt. With PD (hstore), including when init-store is +disabled, the file is read on every start, so keep the secret mounted for as long +as the option is set. With init-store disabled, startup also requires this option +or an explicit non-empty `auth.admin_pa`, so do not drop both. + Older images could change signing keys containing spaces or Unicode, and a trailing space could hide the following configuration property. On upgrade, explicitly provide the intended key for those deployments. Tokens signed with diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java index 98f9ea8bc4..8b7feaa40f 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java @@ -130,7 +130,8 @@ private static void registerPrivateActions() { // HugeGraph Reflection.registerFieldsToFilter(StandardAuthenticator.class, "graph"); Reflection.registerMethodsToFilter(StandardAuthenticator.class, "initAdminUser", - "inputPassword", "graph"); + "inputPassword", "graph", "initialAdminPassword", + "readAdminPasswordFile"); Reflection.registerFieldsToFilter(HugeFactoryAuthProxy.class, "PROTECT_METHODS"); Reflection.registerMethodsToFilter(HugeFactoryAuthProxy.class, "genRegisterPrivateActions", "registerClass", "registerPrivateActions", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java index f64ead4fb2..77662b8275 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java @@ -18,8 +18,18 @@ package org.apache.hugegraph.auth; import java.io.Console; +import java.io.IOException; +import java.io.InputStream; import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CodingErrorAction; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -27,6 +37,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.config.ConfigException; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.ServerOptions; @@ -43,12 +54,28 @@ public class StandardAuthenticator implements HugeAuthenticator { private static final String INITING_STORE = "initing_store"; + private static final int MAX_ADMIN_PA_FILE_BYTES = 4096; + private static final int MAX_ADMIN_PASSWORD_BYTES = 72; private HugeGraph graph = null; - private void initAdminUser() throws Exception { - if (this.requireInitAdminUser()) { - this.initAdminUser(this.inputPassword()); + private void initAdminUser(HugeConfig config) throws Exception { + // init-store only closes the graphs it initialized and skips hstore + // ones, so close the auth graph here even when the password file is + // refused, without letting a close failure hide that error + try { + if (this.requireInitAdminUser()) { + String file = config.get(ServerOptions.ADMIN_PA_FILE); + this.initAdminUser(file.isEmpty() ? this.inputPassword() : + readAdminPasswordFile(file)); + } + } catch (Throwable e) { + try { + this.graph.close(); + } catch (Throwable closeError) { + e.addSuppressed(closeError); + } + throw e; } this.graph.close(); } @@ -202,8 +229,86 @@ public static void initAdminUserIfNeeded(String confFile) throws Exception { config.addProperty(INITING_STORE, true); auth.setup(config); if (auth.graph().backendStoreFeatures().supportsPersistence()) { - auth.initAdminUser(); + auth.initAdminUser(config); + } + } + + /** + * The initial admin password for the bootstrap paths that cannot prompt. + * auth.admin_pa goes through the properties grammar, which trims it, + * processes backslash escapes and decodes it as ISO-8859-1, so + * auth.admin_pa_file takes precedence when both are set. + */ + public static String initialAdminPassword(HugeConfig config) { + String file = config.get(ServerOptions.ADMIN_PA_FILE); + if (!file.isEmpty()) { + return readAdminPasswordFile(file); + } + return config.get(ServerOptions.ADMIN_PA); + } + + private static String readAdminPasswordFile(String file) { + String option = ServerOptions.ADMIN_PA_FILE.name(); + byte[] bytes; + try { + Path path = Paths.get(file); + // Checked before opening, because opening a FIFO blocks until a + // writer connects. Symlinks are followed, so a Kubernetes secret + // mounted through ..data links still counts as a regular file + if (!Files.readAttributes(path, BasicFileAttributes.class) + .isRegularFile()) { + throw new ConfigException("The file '%s' set by '%s' is not a regular file", + file, option); + } + // Bounded, so a path that points at a large file fails quickly + try (InputStream in = Files.newInputStream(path)) { + bytes = in.readNBytes(MAX_ADMIN_PA_FILE_BYTES + 1); + } + } catch (IOException | InvalidPathException e) { + throw new ConfigException("Failed to read the file '%s' set by '%s' (%s)", + e, file, option, e.getClass().getSimpleName()); + } + if (bytes.length > MAX_ADMIN_PA_FILE_BYTES) { + throw new ConfigException("The file '%s' set by '%s' is larger than %s bytes", + file, option, MAX_ADMIN_PA_FILE_BYTES); + } + + String password; + try { + password = StandardCharsets.UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.REPORT) + .onUnmappableCharacter(CodingErrorAction.REPORT) + .decode(ByteBuffer.wrap(bytes)) + .toString(); + } catch (CharacterCodingException e) { + throw new ConfigException("The file '%s' set by '%s' is not valid UTF-8", + e, file, option); + } + + // Some Windows editors and PowerShell prepend a BOM; it is never meant + // as part of the password and would make the admin unreachable + if (password.startsWith("\uFEFF")) { + password = password.substring(1); + } + // Drop the line break that echo, editors and most secret tools append + if (password.endsWith("\r\n")) { + password = password.substring(0, password.length() - 2); + } else if (password.endsWith("\n")) { + password = password.substring(0, password.length() - 1); + } + if (password.isEmpty()) { + throw new ConfigException("The file '%s' set by '%s' is empty", + file, option); + } + // jBCrypt hashes only the first 72 bytes, so a longer password would + // also accept anything that shares its first 72 bytes + if (password.getBytes(StandardCharsets.UTF_8).length > MAX_ADMIN_PASSWORD_BYTES) { + throw new ConfigException("The password in the file '%s' set by '%s' is " + + "longer than %s bytes in UTF-8, the most that " + + "the password hash uses", + file, option, MAX_ADMIN_PASSWORD_BYTES); } + return password; } private class TokenSaslAuthenticator implements SaslNegotiator { diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 285c4b23a5..4fd08f499c 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -480,11 +480,11 @@ public class ServerOptions extends OptionHolder { "" ); - // TODO: accept the initial admin password from the environment (or document the properties - // contract): read through PropertiesConfiguration, the value is trimmed, backslash-unescaped - // and decoded as ISO-8859-1, so the stored password can differ from what the operator set. - // The Helm chart (helm/hugegraph) refuses padded, backslash or non-ASCII admin passwords in - // its schema and Server wrapper; relax that guard once the credential bypasses the file. + // Read through PropertiesConfiguration: the value is trimmed, backslash-unescaped and decoded + // as ISO-8859-1, so leading or trailing spaces, backslashes and characters outside printable + // ASCII need escaping in the file. ADMIN_PA_FILE skips that grammar. The Helm chart + // (helm/hugegraph) still refuses padded, backslash or non-ASCII admin passwords; relax that + // guard once it passes ADMIN_PA_FILE. public static final ConfigOption ADMIN_PA = new ConfigOption<>( "auth.admin_pa", @@ -494,6 +494,22 @@ public class ServerOptions extends OptionHolder { "pa" ); + public static final ConfigOption ADMIN_PA_FILE = + new ConfigOption<>( + "auth.admin_pa_file", + "The path of a regular file (symlinks are followed) " + + "holding the default password for built-in admin " + + "account, read as raw UTF-8 with a leading BOM and one " + + "trailing line break removed. The password may be at " + + "most 72 bytes in UTF-8, the most the hash uses. When " + + "set, it takes precedence over auth.admin_pa and the " + + "init-store prompt; takes effect on first startup. PD " + + "and non-persistent backends read it on every start, so " + + "keep it readable while this option is set.", + null, + "" + ); + public static final ConfigOption SSL_KEYSTORE_FILE = new ConfigOption<>( "ssl.keystore_file", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index c7d821bf38..dea872ef15 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -374,7 +374,8 @@ private void loadMetaFromPD() { this.initK8sManagerIfNeeded(conf); if (shouldBootstrapAdmin(this.authenticator, conf.get(ServerOptions.AUTH_REMOTE_URL))) { - this.initAdminUserIfNeeded(conf.get(ServerOptions.ADMIN_PA)); + this.initAdminUserIfNeeded( + StandardAuthenticator.initialAdminPassword(conf)); } this.createDefaultGraphSpaceIfNeeded(conf); @@ -394,6 +395,18 @@ private static boolean shouldBootstrapAdmin(HugeAuthenticator authenticator, remoteUrl.isEmpty(); } + /** + * With auth.remote_url the built-in authenticator switches to a remote + * AuthManager and never creates a local admin, so the admin password, and + * a password file that may not even be mounted, must not be resolved. + * Custom authenticators keep receiving initAdminUser() as before. + */ + private static boolean usesRemoteAuth(HugeAuthenticator authenticator, + String remoteUrl) { + return authenticator instanceof StandardAuthenticator && + !remoteUrl.isEmpty(); + } + /** * Creates the built-in admin account in PD metadata. With init-store * disabled this is the only bootstrap that admin gets, and init-store's @@ -1876,14 +1889,18 @@ private void checkBackendVersionOrExit(HugeConfig config) { assert hugegraph != null; if (!hugegraph.backendStoreFeatures().supportsPersistence()) { hugegraph.initBackend(); - if (this.requireAuthentication()) { - String adminPassword = config.get(ServerOptions.ADMIN_PA); + if (this.requireAuthentication() && + !usesRemoteAuth(this.authenticator, + config.get(ServerOptions.AUTH_REMOTE_URL))) { + // Outside the try so a bad password file names itself + String adminPassword = + StandardAuthenticator.initialAdminPassword(config); try { this.authenticator().initAdminUser(adminPassword); } catch (Exception e) { throw new BackendException( "The backend store of '%s' can't " + - "initialize admin user", hugegraph.spaceGraphName()); + "initialize admin user", e, hugegraph.spaceGraphName()); } } } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index daeb092b77..92638eea4c 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -21,6 +21,11 @@ arthas.disabled_commands=jad #auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator # for admin password, By default, it is pa and takes effect upon the first startup #auth.admin_pa=pa +# or read it from a file as raw UTF-8 (a leading BOM and one trailing line break are dropped), so +# spaces, backslashes and non-ASCII characters need no escaping; overrides auth.admin_pa. +# It must be a regular file (symlinks are followed) holding at most 72 UTF-8 bytes. +# PD and non-persistent backends read it on every start, so keep it readable while set +#auth.admin_pa_file=/run/secrets/hugegraph-admin-password #auth.graph_store=hugegraph # use pd diff --git a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java index 5362a1821d..517c9d5da5 100644 --- a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java +++ b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java @@ -223,13 +223,22 @@ private static void checkAdminBootstrapReachable(HugeConfig conf, "', not 'hstore'"); } + // A password file is read now, so an unreadable or empty one fails + // here instead of after the server has started + if (!conf.get(ServerOptions.ADMIN_PA_FILE).isEmpty()) { + StandardAuthenticator.initialAdminPassword(conf); + return; + } + // The server creates the admin from this value and cannot prompt for // it, and Docker PASSWORD never reaches this path. An absent or empty // one would hand out the public 'pa' default, so fail instead. Checked // with containsKey because the default is not a configured secret. if (!conf.containsKey(ServerOptions.ADMIN_PA.name()) || conf.get(ServerOptions.ADMIN_PA).isEmpty()) { - throw unreachableAdmin(restConf, "no explicit non-empty '" + + throw unreachableAdmin(restConf, "neither '" + + ServerOptions.ADMIN_PA_FILE.name() + + "' nor an explicit non-empty '" + ServerOptions.ADMIN_PA.name() + "' is configured, so the admin " + "would be created with the " + diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 3cd65b0d8f..1a31cba837 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -50,6 +50,7 @@ import org.apache.hugegraph.unit.api.gremlin.GremlinQueryAPITest; import org.apache.hugegraph.unit.api.space.GraphSpaceAPITest; import org.apache.hugegraph.unit.api.space.SchemaTemplateAPITest; +import org.apache.hugegraph.unit.auth.AdminPasswordFileTest; import org.apache.hugegraph.unit.auth.HugeGraphAuthProxyTest; import org.apache.hugegraph.unit.cache.CacheManagerTest; import org.apache.hugegraph.unit.cache.CacheTest; @@ -227,6 +228,7 @@ ServerInfoManagerTest.class, TaskSchedulerServerInfoTest.class, HugeGraphAuthProxyTest.class, + AdminPasswordFileTest.class, SchemaElementTest.class, HugeGraphTestInfrastructureTest.class, HugeElementViewTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java new file mode 100644 index 0000000000..071d575605 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java @@ -0,0 +1,343 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.unit.auth; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Comparator; +import java.util.stream.Stream; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.auth.HugeUser; +import org.apache.hugegraph.auth.StandardAuthManager; +import org.apache.hugegraph.auth.StandardAuthenticator; +import org.apache.hugegraph.config.ConfigException; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.ServerOptions; +import org.apache.hugegraph.dist.RegisterUtil; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; +import org.apache.hugegraph.util.StringEncoding; +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +/** + * auth.admin_pa goes through the properties grammar, so a padded, escaped or + * non-ASCII password can reach the server changed. auth.admin_pa_file is read + * as raw UTF-8 and must hand back exactly what the operator wrote. + */ +public class AdminPasswordFileTest { + + private Path workDir; + + @BeforeClass + public static void registerOptions() { + RegisterUtil.registerServer(); + } + + @Before + public void setup() throws IOException { + this.workDir = Files.createTempDirectory("admin-password-file-test"); + } + + @After + public void teardown() throws IOException { + try (Stream paths = Files.walk(this.workDir)) { + for (Path path : paths.sorted(Comparator.reverseOrder()) + .toArray(Path[]::new)) { + Files.deleteIfExists(path); + } + } + } + + @Test + public void testFallsBackToAdminPa() { + Assert.assertEquals("pa", initialAdminPassword(null, null)); + Assert.assertEquals("secret", initialAdminPassword(null, "secret")); + } + + @Test + public void testFileKeepsCharactersThePropertiesReaderChanges() + throws IOException { + for (String password : Arrays.asList(" padded ", "two\\back", + "x\ty", "pässword", + "密码 ☃", "a:b=c#d")) { + Path file = this.write(password.getBytes(StandardCharsets.UTF_8)); + Assert.assertEquals(password, initialAdminPassword(file, null)); + } + } + + @Test + public void testFileTakesPrecedenceOverAdminPa() throws IOException { + Path file = this.write("from-file".getBytes(StandardCharsets.UTF_8)); + Assert.assertEquals("from-file", initialAdminPassword(file, "from-pa")); + } + + @Test + public void testFileDropsOneTrailingLineBreak() throws IOException { + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("secret\n")), null)); + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("secret\r\n")), null)); + Assert.assertEquals("secret\n", initialAdminPassword( + this.write(utf8("secret\n\n")), null)); + Assert.assertEquals("secret\r", initialAdminPassword( + this.write(utf8("secret\r")), null)); + } + + @Test + public void testEmptyFileFails() throws IOException { + for (String content : Arrays.asList("", "\n", "\r\n")) { + Path file = this.write(utf8(content)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("is empty", e.getMessage())); + } + } + + @Test + public void testMissingFileFails() { + Path file = this.workDir.resolve("missing"); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("Failed to read", e.getMessage())); + } + + /** + * Guessing another charset is what turned 'pässword' into 'pässword' + * in the first place, so bytes that are not UTF-8 are refused. + */ + @Test + public void testNonUtf8FileFails() throws IOException { + Path file = this.write("pässword".getBytes(StandardCharsets.ISO_8859_1)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, null); + }, e -> Assert.assertContains("not valid UTF-8", e.getMessage())); + } + + /** + * PowerShell and some Windows editors write a BOM before UTF-8 text. Kept, + * it would become an invisible first character of the admin password. + */ + @Test + public void testFileDropsLeadingBom() throws IOException { + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("\ufeffsecret\r\n")), null)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(this.write(utf8("\ufeff\n")), "secret"); + }, e -> Assert.assertContains("is empty", e.getMessage())); + } + + /** + * A path pointing at a large file must not be read whole into memory. + */ + @Test + public void testOversizedFileFails() throws IOException { + byte[] content = new byte[4097]; + Arrays.fill(content, (byte) 'a'); + Path file = this.write(content); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("larger than 4096 bytes", e.getMessage())); + } + + /** + * jBCrypt hashes only the first 72 bytes, so a longer password would also + * accept every password sharing that prefix. The limit counts UTF-8 bytes + * after the BOM and line break are dropped. + */ + @Test + public void testPasswordOverBcryptLimitFails() throws IOException { + String ascii = "a".repeat(72); + String umlauts = "\u00e4".repeat(36); + Assert.assertEquals(ascii, initialAdminPassword( + this.write(utf8("\uFEFF" + ascii + "\r\n")), null)); + Assert.assertEquals(umlauts, initialAdminPassword( + this.write(utf8(umlauts)), null)); + + for (String password : Arrays.asList(ascii + "a", umlauts + "a")) { + Path file = this.write(utf8(password)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("longer than 72 bytes", e.getMessage())); + } + } + + @Test + public void testDirectoryFails() { + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(this.workDir, "secret"); + }, e -> Assert.assertContains("not a regular file", e.getMessage())); + } + + /** + * Opening a FIFO blocks until a writer connects, so it must be refused + * from its attributes before it is opened. The timeout turns a + * regression into a failure instead of a hung build. + */ + @Test(timeout = 10000) + public void testFifoFailsWithoutBlocking() throws Exception { + Path fifo = this.workDir.resolve("fifo"); + int exit; + try { + exit = new ProcessBuilder("mkfifo", fifo.toString()).start() + .waitFor(); + } catch (IOException e) { + exit = -1; + } + Assume.assumeTrue("mkfifo is not available", exit == 0); + + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(fifo, "secret"); + }, e -> Assert.assertContains("not a regular file", e.getMessage())); + } + + /** + * Kubernetes mounts each Secret key as a symlink into a ..data directory, + * so the regular-file check has to follow links. + */ + @Test + public void testSymlinkToRegularFileIsFollowed() throws IOException { + Path target = this.write(utf8("secret\n")); + Path link = this.workDir.resolve("link"); + try { + Files.createSymbolicLink(link, target); + } catch (UnsupportedOperationException | IOException e) { + Assume.assumeNoException("symlinks are not supported", e); + } + Assert.assertEquals("secret", initialAdminPassword(link, null)); + } + + /** + * init-store prompts on stdin for a persistent backend. With a password + * file configured it must create the admin from the file instead. + */ + @Test + public void testInitStoreTakesFileInsteadOfPrompt() throws Exception { + Path file = this.write(utf8("p\u00e4ssword\n")); + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = mockAuthGraph(authManager); + + initStoreAdminUser(graph, config(file, null)); + + ArgumentCaptor admin = ArgumentCaptor.forClass(HugeUser.class); + Mockito.verify(authManager).createUser(admin.capture()); + Assert.assertTrue(StringEncoding.checkPassword("p\u00e4ssword", + admin.getValue().password())); + Mockito.verify(graph).close(); + } + + /** + * init-store does not close an hstore auth graph itself, so a refused + * file must still close it rather than leave the backend open on exit. + */ + @Test + public void testInitStoreClosesGraphWhenFileIsRefused() throws Exception { + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = mockAuthGraph(authManager); + HugeConfig config = config(this.workDir.resolve("missing"), "secret"); + + Assert.assertThrows(ConfigException.class, () -> { + initStoreAdminUser(graph, config); + }, e -> Assert.assertContains("Failed to read", e.getMessage())); + Mockito.verify(authManager, Mockito.never()).createUser(Mockito.any()); + Mockito.verify(graph).close(); + } + + /** + * The refused file is what the operator has to fix, so a failure while + * closing the graph afterwards must not replace it. + */ + @Test + public void testInitStoreCloseFailureKeepsFileError() throws Exception { + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = mockAuthGraph(authManager); + IllegalStateException closeError = new IllegalStateException("close"); + Mockito.doThrow(closeError).when(graph).close(); + HugeConfig config = config(this.workDir.resolve("missing"), "secret"); + + Assert.assertThrows(ConfigException.class, () -> { + initStoreAdminUser(graph, config); + }, e -> { + Assert.assertContains("Failed to read", e.getMessage()); + Assert.assertEquals(closeError, e.getSuppressed()[0]); + }); + } + + private static HugeGraph mockAuthGraph(StandardAuthManager authManager) { + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(graph.hugegraph()).thenReturn(graph); + Mockito.when(graph.authManager()).thenReturn(authManager); + return graph; + } + + /** + * stdin is empty, so a regression that falls back to the prompt fails + * with NoSuchElementException instead of blocking the test run. + */ + private static void initStoreAdminUser(HugeGraph graph, HugeConfig config) { + StandardAuthenticator authenticator = new StandardAuthenticator(); + Whitebox.setInternalState(authenticator, "graph", graph); + InputStream stdin = System.in; + System.setIn(new ByteArrayInputStream(new byte[0])); + try { + Whitebox.invoke(StandardAuthenticator.class, + new Class[]{HugeConfig.class}, "initAdminUser", + authenticator, config); + } finally { + System.setIn(stdin); + } + } + + private static byte[] utf8(String value) { + return value.getBytes(StandardCharsets.UTF_8); + } + + private Path write(byte[] content) throws IOException { + Path file = Files.createTempFile(this.workDir, "admin", ".pa"); + Files.write(file, content); + return file; + } + + private static String initialAdminPassword(Path file, String adminPa) { + return StandardAuthenticator.initialAdminPassword(config(file, adminPa)); + } + + private static HugeConfig config(Path file, String adminPa) { + PropertiesConfiguration properties = new PropertiesConfiguration(); + if (file != null) { + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + file.toString()); + } + if (adminPa != null) { + properties.setProperty(ServerOptions.ADMIN_PA.name(), adminPa); + } + return new HugeConfig(properties); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java index ba88ec7224..fb83fcbe9e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java @@ -395,6 +395,47 @@ public void testDisabledInitStoreRejectsDefaultAdminPassword() } } + @Test + public void testDisabledInitStoreAllowsAdminPasswordFile() + throws Exception { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + Path passwordFile = this.workDir.resolve("admin.pa"); + Files.write(passwordFile, "p\u00e4ssword\n".getBytes(StandardCharsets.UTF_8)); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true", + ServerOptions.ADMIN_PA_FILE.name() + "=" + passwordFile); + + InitStore.main(new String[]{restConf}); + } + + /** + * The server would only read the file after it starts, so init-store + * reads it first and fails on a file the server could not use. + */ + @Test + public void testDisabledInitStoreRejectsUnreadableAdminPasswordFile() + throws IOException { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true", + ServerOptions.ADMIN_PA.name() + "=secret", + ServerOptions.ADMIN_PA_FILE.name() + "=" + + this.workDir.resolve("missing.pa")); + + Assert.assertThrows(ConfigException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains(ServerOptions.ADMIN_PA_FILE.name(), + e.getMessage())); + } + /** * Remote auth delegates to another service, so there is no local admin to * create and the check above must not fire. diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java index 490ffb9920..18e09d98de 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java @@ -18,15 +18,23 @@ package org.apache.hugegraph.unit.core; import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.exception.HugeException; import org.apache.hugegraph.auth.HugeAuthenticator; import org.apache.hugegraph.auth.HugeUser; import org.apache.hugegraph.auth.StandardAuthenticator; +import org.apache.hugegraph.backend.store.BackendFeatures; +import org.apache.hugegraph.backend.store.BackendStoreInfo; +import org.apache.hugegraph.config.ConfigException; import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.ServerOptions; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.event.EventHub; import org.apache.hugegraph.meta.MetaDriver; @@ -35,6 +43,7 @@ import org.apache.hugegraph.meta.managers.SpaceMetaManager; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; +import org.apache.tinkerpop.gremlin.structure.Graph; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -154,6 +163,114 @@ public void testAdminBootstrapOnlyUsesLocalStandardAuthenticator() { Mockito.mock(HugeAuthenticator.class), "")); } + /** + * Non-persistent backends bootstrap the admin on every start, so + * auth.admin_pa_file has to reach that path as well as the PD one. + */ + @Test + public void testNonPersistentBackendTakesAdminPasswordFile() + throws Exception { + Path file = Files.createTempFile("admin-init-test", ".pa"); + try { + Files.write(file, "p\u00e4ss word\n".getBytes(StandardCharsets.UTF_8)); + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth(); + + this.checkBackendVersionOrExit(adminPaFileConfig(file)); + + Mockito.verify(authenticator).initAdminUser("p\u00e4ss word"); + } finally { + Files.deleteIfExists(file); + } + } + + /** + * A refused file must abort startup with its own message rather than be + * folded into the generic backend error or fall back to auth.admin_pa. + */ + @Test + public void testNonPersistentBackendRefusesMissingAdminPasswordFile() + throws Exception { + Path missing = Files.createTempDirectory("admin-init-test") + .resolve("missing"); + try { + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth(); + HugeConfig config = adminPaFileConfig(missing); + + Assert.assertThrows(ConfigException.class, () -> { + this.checkBackendVersionOrExit(config); + }, e -> Assert.assertContains(ServerOptions.ADMIN_PA_FILE.name(), + e.getMessage())); + Mockito.verify(authenticator, Mockito.never()) + .initAdminUser(Mockito.anyString()); + } finally { + Files.deleteIfExists(missing.getParent()); + } + } + + /** + * Remote auth keeps the admin in another service, so an unused password + * file that is missing must not stop a non-persistent backend from + * starting, matching the guard on the PD startup path. + */ + @Test + public void testNonPersistentBackendWithRemoteAuthSkipsAdminPasswordFile() + throws Exception { + Path missing = Files.createTempDirectory("admin-init-test") + .resolve("missing"); + try { + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth( + StandardAuthenticator.class); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + missing.toString()); + properties.setProperty(ServerOptions.AUTH_REMOTE_URL.name(), + "127.0.0.1:8899"); + + this.checkBackendVersionOrExit(new HugeConfig(properties)); + + Mockito.verify(authenticator, Mockito.never()) + .initAdminUser(Mockito.anyString()); + } finally { + Files.deleteIfExists(missing.getParent()); + } + } + + private HugeAuthenticator mockMemoryGraphWithAuth() { + return this.mockMemoryGraphWithAuth(HugeAuthenticator.class); + } + + private HugeAuthenticator mockMemoryGraphWithAuth( + Class authenticatorClass) { + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(graph.backendStoreFeatures()) + .thenReturn(Mockito.mock(BackendFeatures.class)); + BackendStoreInfo info = Mockito.mock(BackendStoreInfo.class); + Mockito.when(info.exists()).thenReturn(true); + Mockito.when(info.checkVersion()).thenReturn(true); + Mockito.when(graph.backendStoreInfo()).thenReturn(info); + Map graphs = Whitebox.getInternalState(this.manager, + "graphs"); + graphs.put("DEFAULT-memory", graph); + + HugeAuthenticator authenticator = Mockito.mock(authenticatorClass); + Mockito.when(authenticator.requireAuthentication()).thenReturn(true); + Whitebox.setInternalState(this.manager, "authenticator", authenticator); + return authenticator; + } + + private void checkBackendVersionOrExit(HugeConfig config) { + Whitebox.invoke(GraphManager.class, new Class[]{HugeConfig.class}, + "checkBackendVersionOrExit", this.manager, config); + } + + private static HugeConfig adminPaFileConfig(Path file) { + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + file.toString()); + properties.setProperty(ServerOptions.ADMIN_PA.name(), "from-pa"); + return new HugeConfig(properties); + } + private static Object swapMetaManagerField(String field, Object replacement) throws Exception {