Generate default password during the plugin's init step
During the plugin's init step one can either rely on that the default
password (16 bytes long) under '$site/data/@PLUGIN@/default.key' file
is being generated, provide own configuration or add a new one under
new 'key-id' (that will be marked as default).
Bug: Issue 16192
Change-Id: I99e4024ecf19d828deb2d84b40fe1dd7e1abe23d
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
index a826886..7ef81d1 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.oauth;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
+
import com.google.common.base.CharMatcher;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
@@ -38,13 +40,9 @@
import javax.servlet.http.HttpServletRequest;
import lombok.Getter;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class GitHubOAuthConfig {
- private static Logger logger = LoggerFactory.getLogger(GitHubOAuthConfig.class);
-
private final Config config;
private final CanonicalWebUrl canonicalWebUrl;
@@ -138,25 +136,16 @@
config.getSubsections(CONF_KEY_SECTION).stream()
.map(KeyConfig::new)
.collect(Collectors.toMap(KeyConfig::getKeyId, Function.identity()));
-
- if (configuredKeyConfig.isEmpty()) {
- logger.warn(
- "No configured '{}' sections found. Default configuration should NOT be used in production code.",
- CONF_KEY_SECTION);
- currentKeyConfig = new KeyConfig();
- keyConfigMap = Map.of(currentKeyConfig.getKeyId(), currentKeyConfig);
- } else {
- keyConfigMap = configuredKeyConfig;
- List<KeyConfig> currentKeyConfigs =
- keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList());
- if (currentKeyConfigs.size() != 1) {
- throw new IllegalStateException(
- String.format(
- "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
- CONF_KEY_SECTION, currentKeyConfigs.size()));
- }
- currentKeyConfig = currentKeyConfigs.get(0);
+ keyConfigMap = configuredKeyConfig;
+ List<KeyConfig> currentKeyConfigs =
+ keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList());
+ if (currentKeyConfigs.size() != 1) {
+ throw new IllegalStateException(
+ String.format(
+ "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
+ CONF_KEY_SECTION, currentKeyConfigs.size()));
}
+ currentKeyConfig = currentKeyConfigs.get(0);
}
public String getOAuthFinalRedirectUrl(HttpServletRequest req) {
@@ -220,7 +209,6 @@
public class KeyConfig {
- public static final String PASSWORD_DEVICE_DEFAULT = "/dev/zero";
public static final int PASSWORD_LENGTH_DEFAULT = 16;
public static final String CIPHER_ALGORITHM_DEFAULT = "AES/ECB/PKCS5Padding";
public static final String SECRET_KEY_ALGORITHM_DEFAULT = "AES";
@@ -251,11 +239,7 @@
CONF_KEY_SECTION, keyId, KEY_DELIMITER));
}
- this.passwordDevice =
- trimTrailingSlash(
- MoreObjects.firstNonNull(
- config.getString(CONF_KEY_SECTION, keyId, PASSWORD_DEVICE_CONFIG_LABEL),
- PASSWORD_DEVICE_DEFAULT));
+ this.passwordDevice = trimTrailingSlash(getPasswordDeviceOrThrow(config, keyId));
this.passwordLength =
config.getInt(
CONF_KEY_SECTION, keyId, PASSWORD_LENGTH_CONFIG_LABEL, PASSWORD_LENGTH_DEFAULT);
@@ -274,15 +258,6 @@
this.keyId = keyId;
}
- private KeyConfig() {
- passwordDevice = PASSWORD_DEVICE_DEFAULT;
- passwordLength = PASSWORD_LENGTH_DEFAULT;
- isCurrent = true;
- cipherAlgorithm = CIPHER_ALGORITHM_DEFAULT;
- keyId = KEY_ID_DEFAULT;
- secretKeyAlgorithm = SECRET_KEY_ALGORITHM_DEFAULT;
- }
-
public byte[] readPassword() throws IOException {
Path devicePath = Paths.get(passwordDevice);
try (FileInputStream in = new FileInputStream(devicePath.toFile())) {
@@ -311,4 +286,22 @@
return keyId;
}
}
+
+ /**
+ * Method returns the password device value for a given {@code keyId}.
+ *
+ * @throws {@link IllegalStateException} when password device is not configured for {@code keyId}
+ */
+ private static String getPasswordDeviceOrThrow(Config config, String keyId) {
+ String passwordDevice =
+ config.getString(CONF_KEY_SECTION, keyId, KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL);
+ if (Strings.isNullOrEmpty(passwordDevice)) {
+ throw new IllegalStateException(
+ String.format(
+ "Configuration error. Missing %s.%s for key-id '%s'",
+ CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, keyId));
+ }
+
+ return passwordDevice;
+ }
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java
new file mode 100644
index 0000000..2f36425
--- /dev/null
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java
@@ -0,0 +1,99 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed 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 com.googlesource.gerrit.plugins.github.oauth;
+
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT;
+import static java.util.Objects.requireNonNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.security.SecureRandom;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PasswordGenerator {
+ private static final Logger logger = LoggerFactory.getLogger(PasswordGenerator.class);
+ public static final String DEFAULT_PASSWORD_FILE = "default.key";
+
+ /**
+ * Generates default password and stores under given {@code Path}. Note that if password already
+ * exists it is not regenerated.
+ *
+ * @param passwordFilePath path that should contain the default password; cannot be {@code null}
+ * @throws {@link IllegalStateException} when file denoted by given {@code Path} is a directory,
+ * cannot be read, has invalid length or doesn't exist and cannot be created
+ * @return {@code true} if password was generated, {@code false} if it already exists
+ */
+ public boolean generate(Path passwordFilePath) {
+ requireNonNull(passwordFilePath);
+
+ File passwordFile = passwordFilePath.toFile();
+
+ if (passwordFile.isDirectory()) {
+ throw logErrorAndCreateRuntimeException(
+ "'%s' is directory whilst a regular file was expected.", passwordFilePath);
+ }
+
+ if (passwordFile.isFile()) {
+ if (!passwordFile.canRead()) {
+ throw logErrorAndCreateRuntimeException(
+ "'%s' password file exists, but cannot be read.", passwordFilePath);
+ }
+
+ long length = passwordFile.length();
+ if (length != PASSWORD_LENGTH_DEFAULT) {
+ throw logErrorAndCreateRuntimeException(
+ "'%s' password file exists but has an invalid length of %d bytes. The expected length is %d bytes.",
+ passwordFilePath, length, PASSWORD_LENGTH_DEFAULT);
+ }
+ return false;
+ }
+
+ byte[] token = generateToken();
+ try {
+ Files.write(passwordFilePath, token);
+ logger.info("Password was stored in {} file", passwordFilePath);
+ return true;
+ } catch (IOException e) {
+ throw logErrorAndCreateRuntimeException(e, "Password generation has failed");
+ }
+ }
+
+ private byte[] generateToken() {
+ SecureRandom random = new SecureRandom();
+ byte[] token = new byte[PASSWORD_LENGTH_DEFAULT];
+ random.nextBytes(token);
+ return token;
+ }
+
+ private IllegalStateException logErrorAndCreateRuntimeException(
+ String msg, Object... parameters) {
+ return logErrorAndCreateRuntimeException(null, msg, parameters);
+ }
+
+ private IllegalStateException logErrorAndCreateRuntimeException(
+ Exception e, String msg, Object... parameters) {
+ String log = String.format(msg, parameters);
+ if (e != null) {
+ logger.error(log, e);
+ return new IllegalStateException(log, e);
+ }
+
+ logger.error(log);
+ return new IllegalStateException(log);
+ }
+}
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java
index 94d1a43..a5bf767 100644
--- a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java
+++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java
@@ -15,12 +15,10 @@
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_KEY_SECTION;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_SECTION;
-import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGORITHM_DEFAULT;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_DELIMITER;
-import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT;
-import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_ALGORITHM_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
@@ -38,6 +36,7 @@
CanonicalWebUrl canonicalWebUrl;
Config config;
+ private static final String testPasswordDevice = "/dev/zero";
@Before
public void setUp() {
@@ -61,28 +60,13 @@
}
@Test
- public void shouldDefaultKeyConfigWhenNoSpecificConfigurationIsSet() {
- GitHubOAuthConfig objectUnderTest = objectUnderTest();
- assertEquals(objectUnderTest.getCurrentKeyConfig().isCurrent(), true);
- assertEquals(
- objectUnderTest.getCurrentKeyConfig().getCipherAlgorithm(), CIPHER_ALGORITHM_DEFAULT);
- assertEquals(
- objectUnderTest.getCurrentKeyConfig().getSecretKeyAlgorithm(),
- SECRET_KEY_ALGORITHM_DEFAULT);
- assertEquals(objectUnderTest.getCurrentKeyConfig().getKeyId(), KEY_ID_DEFAULT);
- }
-
- @Test
- public void shouldDBeAbleToLookupDefaultKeyWhenNoSpecificConfigurationIsSet() {
- assertEquals(objectUnderTest().getKeyConfig(KEY_ID_DEFAULT).isCurrent(), true);
- }
-
- @Test
public void shouldReadASpecificKeyConfig() {
String keySubsection = "someKeyConfig";
String cipherAlgorithm = "AES/CFB8/NoPadding";
String secretKeyAlgorithm = "DES";
config.setBoolean(CONF_KEY_SECTION, keySubsection, CURRENT_CONFIG_LABEL, true);
+ config.setString(
+ CONF_KEY_SECTION, keySubsection, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
config.setString(CONF_KEY_SECTION, keySubsection, CIPHER_ALGO_CONFIG_LABEL, cipherAlgorithm);
config.setString(CONF_KEY_SECTION, keySubsection, SECRET_KEY_CONFIG_LABEL, secretKeyAlgorithm);
@@ -96,10 +80,16 @@
@Test
public void shouldReturnTheExpectedKeyConfigAsCurrent() {
- config.setBoolean(CONF_KEY_SECTION, "currentKeyConfig", CURRENT_CONFIG_LABEL, true);
- config.setBoolean(CONF_KEY_SECTION, "someOtherKeyConfig", CURRENT_CONFIG_LABEL, false);
+ String currentKeyConfig = "currentKeyConfig";
+ String someOtherKeyConfig = "someOtherKeyConfig";
+ config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true);
+ config.setString(
+ CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
+ config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false);
+ config.setString(
+ CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
- assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), "currentKeyConfig");
+ assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), currentKeyConfig);
}
@Test
@@ -107,7 +97,11 @@
String currentKeyConfig = "currentKeyConfig";
String someOtherKeyConfig = "someOtherKeyConfig";
config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true);
+ config.setString(
+ CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false);
+ config.setString(
+ CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
GitHubOAuthConfig objectUnderTest = objectUnderTest();
@@ -116,6 +110,18 @@
}
@Test
+ public void shouldThrowWhenNoKeyIdIsConfigured() {
+ IllegalStateException illegalStateException =
+ assertThrows(IllegalStateException.class, this::objectUnderTest);
+
+ assertEquals(
+ illegalStateException.getMessage(),
+ String.format(
+ "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
+ CONF_KEY_SECTION, 0));
+ }
+
+ @Test
public void shouldThrowWhenNoKeyConfigIsSetAsCurrent() {
config.setBoolean(CONF_KEY_SECTION, "someKeyConfig", CURRENT_CONFIG_LABEL, false);
@@ -145,6 +151,21 @@
assertThrows(IllegalStateException.class, this::objectUnderTest);
}
+ @Test
+ public void shouldThrowWhenKeyIdMissesPasswordDevice() {
+ String someKeyConfig = "someKeyConfig";
+ config.setBoolean(CONF_KEY_SECTION, someKeyConfig, CURRENT_CONFIG_LABEL, true);
+
+ IllegalStateException illegalStateException =
+ assertThrows(IllegalStateException.class, this::objectUnderTest);
+
+ assertEquals(
+ String.format(
+ "Configuration error. Missing %s.%s for key-id '%s'",
+ CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, someKeyConfig),
+ illegalStateException.getMessage());
+ }
+
private GitHubOAuthConfig objectUnderTest() {
return new GitHubOAuthConfig(config, canonicalWebUrl);
}
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java
index 9621fc5..3e31d6b 100644
--- a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java
+++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java
@@ -17,6 +17,7 @@
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_SECTION;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL;
import static com.googlesource.gerrit.plugins.github.oauth.OAuthTokenCipher.splitKeyIdFromMaterial;
@@ -31,32 +32,38 @@
import com.google.inject.util.Providers;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
import java.util.Base64;
import java.util.List;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
+import org.junit.ClassRule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
public class OAuthTokenCipherTest {
CanonicalWebUrl canonicalWebUrl;
Config config;
+ @ClassRule public static TemporaryFolder temporaryFolder = new TemporaryFolder();
+
private static final String VERSION1_KEY_ID = "version1";
private static final String VERSION2_KEY_ID = "version2";
@Before
public void setUp() {
- config = new Config();
-
- config.setString(CONF_SECTION, null, "clientSecret", "theSecret");
- config.setString(CONF_SECTION, null, "clientId", "theClientId");
- config.setString("auth", null, "httpHeader", "GITHUB_USER");
- config.setString("auth", null, "type", AuthType.HTTP.toString());
+ config = createCommonConfig();
config.setBoolean(CONF_KEY_SECTION, VERSION1_KEY_ID, CURRENT_CONFIG_LABEL, true);
config.setBoolean(CONF_KEY_SECTION, VERSION2_KEY_ID, CURRENT_CONFIG_LABEL, false);
+ String testPasswordDevice = "/dev/zero";
+ config.setString(
+ CONF_KEY_SECTION, VERSION1_KEY_ID, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
+ config.setString(
+ CONF_KEY_SECTION, VERSION2_KEY_ID, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
+
canonicalWebUrl =
Guice.createInjector(
new AbstractModule() {
@@ -71,10 +78,32 @@
}
@Test
- public void shouldEncryptAndDecryptAToken() throws IOException {
- String someOauthToken = "someToken";
- OAuthTokenCipher objectUnderTest = objectUnderTest();
+ public void shouldEncryptAndDecryptATokenWithPasswordGeneratedAtInit() throws IOException {
+ // simulate plugin init step by generating a password to a file and configuring it in
+ // gerrit.config
+ Path passwordFilePath =
+ temporaryFolder.newFolder().toPath().resolve(PasswordGenerator.DEFAULT_PASSWORD_FILE);
+ new PasswordGenerator().generate(passwordFilePath);
+ config = createCommonConfig();
+ config.setBoolean(CONF_KEY_SECTION, KEY_ID_DEFAULT, CURRENT_CONFIG_LABEL, true);
+ config.setString(
+ CONF_KEY_SECTION,
+ KEY_ID_DEFAULT,
+ PASSWORD_DEVICE_CONFIG_LABEL,
+ passwordFilePath.toString());
+
+ verifyTokenEncryptionAndDecryption(objectUnderTest());
+ }
+
+ @Test
+ public void shouldEncryptAndDecryptAToken() throws IOException {
+ verifyTokenEncryptionAndDecryption(objectUnderTest());
+ }
+
+ private void verifyTokenEncryptionAndDecryption(OAuthTokenCipher objectUnderTest)
+ throws CipherException {
+ String someOauthToken = "someToken";
String encrypt = objectUnderTest.encrypt(someOauthToken);
assertNotEquals(encrypt, someOauthToken);
assertEquals(objectUnderTest.decrypt(encrypt), someOauthToken);
@@ -160,6 +189,19 @@
}
private OAuthTokenCipher objectUnderTest() throws IOException {
- return new OAuthTokenCipher(new GitHubOAuthConfig(config, canonicalWebUrl));
+ return objectUnderTest(config);
+ }
+
+ private OAuthTokenCipher objectUnderTest(Config testConfig) throws IOException {
+ return new OAuthTokenCipher(new GitHubOAuthConfig(testConfig, canonicalWebUrl));
+ }
+
+ private static Config createCommonConfig() {
+ Config config = new Config();
+ config.setString(CONF_SECTION, null, "clientSecret", "theSecret");
+ config.setString(CONF_SECTION, null, "clientId", "theClientId");
+ config.setString("auth", null, "httpHeader", "GITHUB_USER");
+ config.setString("auth", null, "type", AuthType.HTTP.toString());
+ return config;
}
}
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java
new file mode 100644
index 0000000..0fe37a8
--- /dev/null
+++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java
@@ -0,0 +1,92 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed 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 com.googlesource.gerrit.plugins.github.oauth;
+
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class PasswordGeneratorTest {
+ @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private Path passwordPath;
+ private final PasswordGenerator objectUnderTest = new PasswordGenerator();
+
+ @Before
+ public void setup() throws IOException {
+ passwordPath =
+ temporaryFolder.newFolder().toPath().resolve(PasswordGenerator.DEFAULT_PASSWORD_FILE);
+ }
+
+ @Test
+ public void shouldGenerateKeyFileWithPasswordDefaultLength() throws IOException {
+ assertTrue(objectUnderTest.generate(passwordPath));
+ assertTrue(Files.isRegularFile(passwordPath));
+
+ byte[] token = Files.readAllBytes(passwordPath);
+ assertEquals(
+ String.format(
+ "Generated password length doesn't equal to expected %d", PASSWORD_LENGTH_DEFAULT),
+ token.length,
+ PASSWORD_LENGTH_DEFAULT);
+ }
+
+ @Test
+ public void shouldNotGenerateNewDefaultKeyIfOneAlreadyExistAndIsNotEmpty() throws IOException {
+ assertTrue(objectUnderTest.generate(passwordPath));
+ byte[] expected = Files.readAllBytes(passwordPath);
+ assertFalse(objectUnderTest.generate(passwordPath));
+ byte[] token = Files.readAllBytes(passwordPath);
+ assertArrayEquals("Existing password file was overwritten", expected, token);
+ }
+
+ @Test
+ public void shouldGenerateDifferentContentForDifferentSites() throws IOException {
+ assertTrue(objectUnderTest.generate(passwordPath));
+ byte[] siteA = Files.readAllBytes(passwordPath);
+
+ assertTrue(passwordPath.toFile().delete());
+ assertTrue(objectUnderTest.generate(passwordPath));
+ byte[] siteB = Files.readAllBytes(passwordPath);
+ assertFalse(
+ "The same password was generated for two different sites", Arrays.equals(siteA, siteB));
+ }
+
+ @Test
+ public void shouldThrowIllegalStateExceptionWhenDefaultKeyIsDirectory() throws IOException {
+ // create dir from passwordPath
+ assertTrue(passwordPath.toFile().mkdir());
+
+ IllegalStateException illegalStateException =
+ assertThrows(IllegalStateException.class, () -> objectUnderTest.generate(passwordPath));
+
+ assertTrue(
+ illegalStateException
+ .getMessage()
+ .endsWith("is directory whilst a regular file was expected."));
+ }
+}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
index 828613e..894e1a5 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
@@ -13,15 +13,35 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_KEY_SECTION;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGORITHM_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.IS_CURRENT_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_ALGORITHM_DEFAULT;
+import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL;
+import static com.googlesource.gerrit.plugins.github.oauth.PasswordGenerator.DEFAULT_PASSWORD_FILE;
+
import com.google.common.base.Strings;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.client.AuthType;
import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.InitUtil;
import com.google.gerrit.pgm.init.api.Section;
+import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.github.oauth.PasswordGenerator;
import java.net.URISyntaxException;
+import java.nio.file.Path;
import java.util.EnumSet;
+import java.util.Optional;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
public class InitGitHub implements InitStep {
private static final String GITHUB_URL = "https://github.com";
@@ -29,15 +49,26 @@
private static final String GITHUB_REGISTER_APPLICATION_PATH = "/settings/applications/new";
private static final String GERRIT_OAUTH_CALLBACK_PATH = "oauth";
+ private final Path pluginData;
private final ConsoleUI ui;
private final Section auth;
private final Section httpd;
private final Section github;
private final Section gerrit;
+ private final Section.Factory sections;
+ private final FileBasedConfig cfg;
@Inject
- InitGitHub(final ConsoleUI ui, final Section.Factory sections) {
+ InitGitHub(
+ @PluginName String pluginName,
+ SitePaths site,
+ final ConsoleUI ui,
+ final Section.Factory sections,
+ InitFlags flags) {
+ this.pluginData = site.data_dir.resolve(pluginName);
this.ui = ui;
+ this.sections = sections;
+ this.cfg = flags.cfg;
this.github = sections.get("github", null);
this.httpd = sections.get("httpd", null);
this.auth = sections.get("auth", null);
@@ -85,6 +116,87 @@
httpd.unset("filterClass");
httpd.unset("httpHeader");
}
+
+ setupGitHubOAuthTokenCipher();
+ }
+
+ private void setupGitHubOAuthTokenCipher() {
+ ui.header("GitHub OAuth token cipher configuration");
+ Optional<String> maybeCurrentKeyId = getCurrentKeyId();
+
+ boolean configureNewPasswordDevice = false;
+ if (maybeCurrentKeyId.isPresent()) {
+ if (!ui.yesno(
+ false,
+ "Current GitHub OAuth token cipher is configured under the %s key id. Do you want to configure a new one?",
+ maybeCurrentKeyId.get())) {
+ return;
+ }
+
+ String notUniqueKeyIdPrefix = "K";
+ do {
+ String newKeyId = ui.readString("some-key-id", "%sey identifier", notUniqueKeyIdPrefix);
+ if (cfg.getSubsections(CONF_KEY_SECTION).contains(newKeyId)) {
+ notUniqueKeyIdPrefix =
+ String.format(
+ "Provided key id '%s' already exists. Please provide a different key.", newKeyId);
+ continue;
+ }
+
+ cfg.setBoolean(CONF_KEY_SECTION, maybeCurrentKeyId.get(), CURRENT_CONFIG_LABEL, false);
+ maybeCurrentKeyId = Optional.of(newKeyId);
+ configureNewPasswordDevice = true;
+ break;
+ } while (true);
+ }
+
+ String currentKeyId = maybeCurrentKeyId.orElse(KEY_ID_DEFAULT);
+ ui.message("Configuring GitHub OAuth token cipher under '%s' key id\n", currentKeyId);
+ Section gitHubKey = sections.get(CONF_KEY_SECTION, currentKeyId);
+
+ gitHubKey.set(CURRENT_CONFIG_LABEL, "true");
+
+ Path defaultPasswordPath = pluginData.resolve(DEFAULT_PASSWORD_FILE);
+ String currentPasswordPath =
+ gitHubKey.string(
+ "Password file or device",
+ PASSWORD_DEVICE_CONFIG_LABEL,
+ configureNewPasswordDevice ? null : defaultPasswordPath.toString());
+ if (defaultPasswordPath.toString().equalsIgnoreCase(currentPasswordPath)) {
+ pluginData.toFile().mkdirs();
+ if (new PasswordGenerator().generate(Path.of(currentPasswordPath))) {
+ ui.message(
+ "New password (%d bytes long) was generated under '%s' file.\n",
+ PASSWORD_LENGTH_DEFAULT, currentPasswordPath);
+ } else {
+ ui.message(
+ "The file under '%s' path already exists. Password wasn't regenerated.\n",
+ currentPasswordPath);
+ }
+ } else {
+ // ask for length only if default password is not used
+ gitHubKey.set(
+ PASSWORD_LENGTH_CONFIG_LABEL,
+ String.valueOf(ui.readInt(PASSWORD_LENGTH_DEFAULT, "Password length in bytes")));
+ }
+
+ gitHubKey.string(
+ "The algorithm to be used to encrypt the provided password",
+ SECRET_KEY_CONFIG_LABEL,
+ SECRET_KEY_ALGORITHM_DEFAULT);
+
+ gitHubKey.string(
+ "The algorithm to be used for encryption/decryption",
+ CIPHER_ALGO_CONFIG_LABEL,
+ CIPHER_ALGORITHM_DEFAULT);
+ }
+
+ private Optional<String> getCurrentKeyId() {
+ return cfg.getSubsections(CONF_KEY_SECTION).stream()
+ .filter(
+ keyId ->
+ cfg.getBoolean(CONF_KEY_SECTION, keyId, CURRENT_CONFIG_LABEL, IS_CURRENT_DEFAULT))
+ .findFirst();
}
private void authSetDefault(String key, String defValue) {
diff --git a/github-plugin/src/main/resources/Documentation/config.md b/github-plugin/src/main/resources/Documentation/config.md
index ea32890..e2ea0a3 100644
--- a/github-plugin/src/main/resources/Documentation/config.md
+++ b/github-plugin/src/main/resources/Documentation/config.md
@@ -7,7 +7,8 @@
library to work properly.
GitHub OAuth library rely on Gerrit HTTP authentication defined during the standard
-Gerrit init steps.
+Gerrit init steps. It also requires GitHub OAuth token cipher configuration
+(details `Key configuration` section) but provides convenient defaults for it.
See below a sample session of relevant init steps for a default
configuration pointing to the Web GitHub instance:
@@ -36,6 +37,15 @@
ClientSecret []: f82c3f9b3802666f2adcc4c8cacfb164295b0a99
confirm password :
HTTP Authentication Header [GITHUB_USER]:
+
+ *** GitHub OAuth token cipher configuration
+ ***
+
+ Configuring GitHub OAuth token cipher under 'current' key id
+ Password file or device [gerrit/data/github-plugin/default.key]:
+ New password (16 bytes long) was generated under 'gerrit/data/github-plugin/default.key' file.
+ The algorithm to be used to encrypt the provided password [AES]:
+ The algorithm to be used for encryption/decryption [AES/ECB/PKCS5Padding]:
```
Configuration
@@ -89,15 +99,14 @@
-------------
Since this plugin obtains credentials from Github and persists them in Gerrit,
-it also takes care of encrypting them at rest. The Gerrit admin can configure
-how this is done by setting the relevant configuration parameters.
+it also takes care of encrypting them at rest. Encryption configuration is a
+mandatory step performed during the plugin init (that also provides convenient
+defaults). The Gerrit admin can introduce its own cipher configuration
+(already in init step) by setting the following parameters.
github-key.<key-id>.passwordDevice
: The device or file where to retrieve the encryption passphrase.\
-Default: /dev/zero
-
-*NOTE*: such configuration is considered insecure and should *not be used in
-production*, always set a non-zero password device for deriving the key.
+This is a required parameter for `key-id` configuration.
github-key.<key-id>.passwordLength
: The length in bytes of the password read from the passwordDevice.\
@@ -111,7 +120,7 @@
Default: AES/ECB/PKCS5Padding
github-key.<key-id>.secretKeyAlgorithm
-: the algorithm to be used to encrypt the provided password. Available
+: The algorithm to be used to encrypt the provided password. Available
algorithms are described in
the [Cipher section](https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#cipher-algorithm-names)
of the Java Cryptography Architecture Standard Algorithm Name Documentation.\
@@ -129,14 +138,12 @@
in the configuration. New credentials will always be encrypted with
the `current` `<key-id>`.
-Note that unencrypted oauth tokens will be handled gracefully and just passed
-through to github by the decryption algorithm. This is done so that oauth tokens
-that were persisted _before_ the encryption feature was implemented will still
-be considered valid until their natural expiration time.
+*Notes:*
+Unencrypted oauth tokens will be handled gracefully and just passed through to
+github by the decryption algorithm. This is done so that oauth tokens that were
+persisted _before_ the encryption feature was implemented will still be
+considered valid until their natural expiration time.
-If no `github-key.<key-id>` exists in configuration, then a default current key
-configuration
-(named `current`) will be inferred, using the defaults documented above.
-
-*NOTE* such configuration is considered insecure and should *not be used in
-production*.
\ No newline at end of file
+Plugin will not start if no `github-key.<key-id>` section, marked as current,
+exists in configuration. One needs to either configure it manually or call init
+for a default configuration to be created.
