Merge branch 'stable-3.5' into stable-3.6 * stable-3.5: Set plugin version to 3.4.7 Fire git ref update events for all imported refs PluginVelocityRuntimeProvider: Fix warning flagged by error prone Fix default scopes resolution Change-Id: I8e937f978898fa9ef806eec81758b07435b21439
diff --git a/github-oauth/pom.xml b/github-oauth/pom.xml index 0b7a9b5..21631aa 100644 --- a/github-oauth/pom.xml +++ b/github-oauth/pom.xml
@@ -21,7 +21,7 @@ <parent> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.5.0.1</version> + <version>3.6.0-rc1</version> </parent> <artifactId>github-oauth</artifactId> <name>Gerrit Code Review - GitHub OAuth login</name> @@ -120,5 +120,10 @@ <version>4.4</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <scope>test</scope> + </dependency> </dependencies> </project>
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/CipherException.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/CipherException.java new file mode 100644 index 0000000..6ab74d0 --- /dev/null +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/CipherException.java
@@ -0,0 +1,34 @@ +// 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 java.io.IOException; + +/** + * Signals that a cipher exception has occurred. This class can be used to represent exception for + * both encryption and decryption failures + */ +public class CipherException extends IOException { + private static final long serialVersionUID = 1L; + + /** + * Constructs a {@code CipherException} with the specified detail message and cause + * + * @param message The detail message of the failure + * @param cause The cause of the failure + */ + public CipherException(String message, Exception cause) { + super(message, cause); + } +}
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 f0799f6..151f636 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,23 +13,29 @@ // 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; import com.google.common.base.Strings; import com.google.gerrit.extensions.client.AuthType; import com.google.gerrit.httpd.CanonicalWebUrl; -import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope; +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import lombok.Getter; @@ -41,9 +47,9 @@ private final CanonicalWebUrl canonicalWebUrl; public static final String CONF_SECTION = "github"; + public static final String CONF_KEY_SECTION = "github-key"; public static final String GITHUB_OAUTH_AUTHORIZE = "/login/oauth/authorize"; public static final String GITHUB_OAUTH_ACCESS_TOKEN = "/login/oauth/access_token"; - public static final String GITHUB_GET_USER = "/user"; public static final String GERRIT_OAUTH_FINAL = "/oauth"; public static final String GITHUB_URL_DEFAULT = "https://github.com"; public static final String GITHUB_API_URL_DEFAULT = "https://api.github.com"; @@ -71,10 +77,11 @@ public final long httpConnectionTimeout; public final long httpReadTimeout; + private final Map<String, KeyConfig> keyConfigMap; + private final KeyConfig currentKeyConfig; @Inject - protected GitHubOAuthConfig( - @GerritServerConfig Config config, CanonicalWebUrl canonicalWebUrl, AuthConfig authConfig) { + protected GitHubOAuthConfig(@GerritServerConfig Config config, CanonicalWebUrl canonicalWebUrl) { this.config = config; this.canonicalWebUrl = canonicalWebUrl; @@ -125,6 +132,21 @@ ConfigUtil.getTimeUnit( config, CONF_SECTION, null, "httpReadTimeout", 30, TimeUnit.SECONDS), TimeUnit.SECONDS); + + Map<String, KeyConfig> configuredKeyConfig = + config.getSubsections(CONF_KEY_SECTION).stream() + .map(KeyConfig::new) + .collect(Collectors.toMap(KeyConfig::getKeyId, Function.identity())); + 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) { @@ -177,4 +199,110 @@ } return scopes.get(GITHUB_DEFAULT_SCOPES_KEY).toArray(new Scope[0]); } + + public KeyConfig getCurrentKeyConfig() { + return currentKeyConfig; + } + + public KeyConfig getKeyConfig(String subsection) { + return keyConfigMap.get(subsection); + } + + public class KeyConfig { + + 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"; + public static final boolean IS_CURRENT_DEFAULT = false; + public static final String KEY_ID_DEFAULT = "current"; + + public static final String PASSWORD_DEVICE_CONFIG_LABEL = "passwordDevice"; + public static final String PASSWORD_LENGTH_CONFIG_LABEL = "passwordLength"; + public static final String SECRET_KEY_CONFIG_LABEL = "secretKeyAlgorithm"; + public static final String CIPHER_ALGO_CONFIG_LABEL = "cipherAlgorithm"; + public static final String CURRENT_CONFIG_LABEL = "current"; + + public static final String KEY_DELIMITER = ":"; + + private final String passwordDevice; + private final Integer passwordLength; + private final String cipherAlgorithm; + private final String secretKeyAlgorithm; + private final String keyId; + private final Boolean isCurrent; + + KeyConfig(String keyId) { + + if (keyId.contains(KEY_DELIMITER)) { + throw new IllegalStateException( + String.format( + "Configuration error. %s.%s should not contain '%s'", + CONF_KEY_SECTION, keyId, KEY_DELIMITER)); + } + + this.passwordDevice = trimTrailingSlash(getPasswordDeviceOrThrow(config, keyId)); + this.passwordLength = + config.getInt( + CONF_KEY_SECTION, keyId, PASSWORD_LENGTH_CONFIG_LABEL, PASSWORD_LENGTH_DEFAULT); + isCurrent = + config.getBoolean(CONF_KEY_SECTION, keyId, CURRENT_CONFIG_LABEL, IS_CURRENT_DEFAULT); + + this.cipherAlgorithm = + MoreObjects.firstNonNull( + config.getString(CONF_KEY_SECTION, keyId, CIPHER_ALGO_CONFIG_LABEL), + CIPHER_ALGORITHM_DEFAULT); + + this.secretKeyAlgorithm = + MoreObjects.firstNonNull( + config.getString(CONF_KEY_SECTION, keyId, SECRET_KEY_CONFIG_LABEL), + SECRET_KEY_ALGORITHM_DEFAULT); + this.keyId = keyId; + } + + public byte[] readPassword() throws IOException { + Path devicePath = Paths.get(passwordDevice); + try (FileInputStream in = new FileInputStream(devicePath.toFile())) { + byte[] passphrase = new byte[passwordLength]; + if (in.read(passphrase) < passwordLength) { + throw new IOException("End of password device has already been reached"); + } + + return passphrase; + } + } + + public String getCipherAlgorithm() { + return cipherAlgorithm; + } + + public String getSecretKeyAlgorithm() { + return secretKeyAlgorithm; + } + + public Boolean isCurrent() { + return isCurrent; + } + + public String getKeyId() { + 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/IdentifiedUserGitHubLoginProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java index b953531..5449347 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
@@ -39,17 +39,20 @@ private final GitHubOAuthConfig config; private final AccountCache accountCache; private final GitHubHttpConnector httpConnector; + private final OAuthTokenCipher oAuthTokenCipher; @Inject public IdentifiedUserGitHubLoginProvider( Provider<IdentifiedUser> identifiedUserProvider, GitHubOAuthConfig config, GitHubHttpConnector httpConnector, - AccountCache accountCache) { + AccountCache accountCache, + OAuthTokenCipher oAuthTokenCipher) { this.userProvider = identifiedUserProvider; this.config = config; this.accountCache = accountCache; this.httpConnector = httpConnector; + this.oAuthTokenCipher = oAuthTokenCipher; } @Override @@ -75,16 +78,17 @@ } } - private AccessToken newAccessTokenFromUser(String username) { + private AccessToken newAccessTokenFromUser(String username) throws IOException { AccountState account = accountCache.getByUsername(username).get(); Collection<ExternalId> externalIds = account.externalIds(); for (ExternalId accountExternalId : externalIds) { String key = accountExternalId.key().get(); if (key.startsWith(EXTERNAL_ID_PREFIX)) { - return new AccessToken(key.substring(EXTERNAL_ID_PREFIX.length())); + String encryptedOauthToken = key.substring(EXTERNAL_ID_PREFIX.length()); + String decryptedOauthToken = oAuthTokenCipher.decrypt(encryptedOauthToken); + return new AccessToken(decryptedOauthToken); } } - return null; } }
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java index 5025002..425755f 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
@@ -29,7 +29,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.slf4j.LoggerFactory; @Singleton
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipher.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipher.java new file mode 100644 index 0000000..a8133dc --- /dev/null +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipher.java
@@ -0,0 +1,168 @@ +// 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.KEY_DELIMITER; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.util.Base64; +import java.util.List; +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.SecretKeySpec; + +/** Provides the ability to encrypt and decrypt an OAuth token */ +@Singleton +public class OAuthTokenCipher { + private final String currentCipherAlgorithm; + private final SecretKeySpec currentSecretKey; + private final String currentKeyId; + private final GitHubOAuthConfig config; + + /** + * Constructs a {@code OAuthTokenCipher} using cipher algorithm specified in configuration + * + * @param config the github oauth configuration object + * @throws IOException when the cipher could not be constructed + */ + @Inject + public OAuthTokenCipher(GitHubOAuthConfig config) throws IOException { + GitHubOAuthConfig.KeyConfig currentKeyConfig = config.getCurrentKeyConfig(); + currentKeyId = currentKeyConfig.getKeyId(); + currentCipherAlgorithm = currentKeyConfig.getCipherAlgorithm(); + currentSecretKey = + new SecretKeySpec( + currentKeyConfig.readPassword(), currentKeyConfig.getSecretKeyAlgorithm()); + this.config = config; + } + + /** + * Encrypts the provided string and returns its base64 representation, prefixed with the name of + * the configuration subsection used to encrypt it, separated by ':'. + * + * <p>For example: + * + * <p>current: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 + * + * @param plainText the string to encrypt + * @return the base64-encoded encrypted string + * @throws CipherException when the string could not be encrypted + */ + public String encrypt(final String plainText) throws CipherException { + try { + return prependCurrentKeyId( + Base64.getEncoder() + .encodeToString( + initCurrentCipherForEncryption() + .doFinal(plainText.getBytes(StandardCharsets.UTF_8)))); + } catch (IllegalBlockSizeException | BadPaddingException e) { + throw new CipherException("Could not encrypt oauth token", e); + } + } + + /** + * Decrypts the provided base64-encoded encrypted string, prefixed with the name of the + * configuration subsection used to encrypt it, separated by ':'. + * + * <p>For example: + * + * <p>current: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 + * + * <p>In order to provide back-compatibility with plaintext oauth tokens that were stored before + * encryption was introduced, it will return the input string as-is, when the string is not + * prefixed by a key-id + * + * @param base64EncryptedString the string to decrypt + * @return the plainText string + * @throws CipherException when the string could not be decrypted + */ + public String decrypt(final String base64EncryptedString) throws CipherException { + try { + + if (isPrefixedWithKeyId(base64EncryptedString)) { + List<String> keyIdAndMaterial = splitKeyIdFromMaterial(base64EncryptedString); + String keyId = keyIdAndMaterial.get(0); + String material = keyIdAndMaterial.get(1); + Cipher decryptCipher = getCipherFor(keyId); + return new String( + decryptCipher.doFinal(Base64.getDecoder().decode(material)), StandardCharsets.UTF_8); + } + return base64EncryptedString; + + } catch (IllegalStateException + | IllegalArgumentException + | IllegalBlockSizeException + | BadPaddingException + | IOException e) { + throw new CipherException("Could not decrypt oauth token", e); + } + } + + private static Cipher initCipher(String cipherAlgorithm, SecretKeySpec secretKey, int mode) + throws CipherException { + try { + Cipher cipher = Cipher.getInstance(cipherAlgorithm); + cipher.init(mode, secretKey); + return cipher; + } catch (NoSuchPaddingException | NoSuchAlgorithmException | InvalidKeyException e) { + throw new CipherException("Could not init cipher", e); + } + } + + private Cipher initCurrentCipherForEncryption() throws CipherException { + return initCipher(currentCipherAlgorithm, currentSecretKey, Cipher.ENCRYPT_MODE); + } + + private String prependCurrentKeyId(String base64EncodedString) { + return String.format("%s%s%s", currentKeyId, KEY_DELIMITER, base64EncodedString); + } + + private static boolean isPrefixedWithKeyId(String maybeEncryptedString) { + return maybeEncryptedString.contains(KEY_DELIMITER); + } + + @VisibleForTesting + static List<String> splitKeyIdFromMaterial(String base64EncryptedString) { + List<String> tokens = Splitter.on(KEY_DELIMITER).splitToList(base64EncryptedString); + int nOfTokens = tokens.size(); + if (nOfTokens != 2) { + throw new IllegalStateException( + String.format( + "The encrypted key is expected to contain 2 tokens (keyId:key), whereas it contains %d tokens", + nOfTokens)); + } + return tokens; + } + + private Cipher getCipherFor(String keyId) throws IOException { + GitHubOAuthConfig.KeyConfig keyConfig = config.getKeyConfig(keyId); + if (keyConfig == null) { + throw new IllegalStateException( + String.format("Could not find key-id '%s' in configuration", keyId)); + } + return initCipher( + keyConfig.getCipherAlgorithm(), + new SecretKeySpec(keyConfig.readPassword(), keyConfig.getSecretKeyAlgorithm()), + Cipher.DECRYPT_MODE); + } +}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java index 316d1bb..3c72141 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java
@@ -31,7 +31,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; @@ -49,6 +49,7 @@ private final SitePaths sites; private final ScopedProvider<GitHubLogin> loginProvider; private final OAuthProtocol oauth; + private final OAuthTokenCipher oAuthTokenCipher; @Inject public OAuthWebFilter( @@ -57,11 +58,13 @@ OAuthProtocol oauth, // We need to explicitly tell Guice the correct implementation // as this filter is instantiated with a standard Gerrit WebModule - GitHubLogin.Provider loginProvider) { + GitHubLogin.Provider loginProvider, + OAuthTokenCipher oAuthTokenCipher) { this.config = config; this.sites = sites; this.oauth = oauth; this.loginProvider = loginProvider; + this.oAuthTokenCipher = oAuthTokenCipher; } @Override @@ -88,13 +91,14 @@ } if (ghLogin != null && ghLogin.isLoggedIn()) { + String hashedToken = oAuthTokenCipher.encrypt(ghLogin.getToken().accessToken); httpRequest = new AuthenticatedHttpRequest( httpRequest, config.httpHeader, ghLogin.getMyself().getLogin(), config.oauthHttpHeader, - GITHUB_EXT_ID + ghLogin.getToken().accessToken); + GITHUB_EXT_ID + hashedToken); } chain.doFilter(httpRequest, httpResponse);
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 new file mode 100644 index 0000000..a5bf767 --- /dev/null +++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java
@@ -0,0 +1,172 @@ +// 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.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_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.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; + +import com.google.gerrit.extensions.client.AuthType; +import com.google.gerrit.httpd.CanonicalWebUrl; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.util.Providers; +import org.eclipse.jgit.lib.Config; +import org.junit.Before; +import org.junit.Test; + +public class GitHubOAuthConfigTest { + + CanonicalWebUrl canonicalWebUrl; + Config config; + private static final String testPasswordDevice = "/dev/zero"; + + @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()); + + canonicalWebUrl = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class) + .annotatedWith(com.google.gerrit.server.config.CanonicalWebUrl.class) + .toProvider(Providers.of(null)); + } + }) + .getInstance(CanonicalWebUrl.class); + } + + @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); + + GitHubOAuthConfig objectUnderTest = objectUnderTest(); + + assertEquals(objectUnderTest.getCurrentKeyConfig().isCurrent(), true); + assertEquals(objectUnderTest.getCurrentKeyConfig().getCipherAlgorithm(), cipherAlgorithm); + assertEquals(objectUnderTest.getCurrentKeyConfig().getSecretKeyAlgorithm(), secretKeyAlgorithm); + assertEquals(objectUnderTest.getCurrentKeyConfig().getKeyId(), keySubsection); + } + + @Test + public void shouldReturnTheExpectedKeyConfigAsCurrent() { + 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); + } + + @Test + public void shouldReadMultipleKeyConfigs() { + 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(); + + assertEquals(objectUnderTest.getKeyConfig(currentKeyConfig).getKeyId(), currentKeyConfig); + assertEquals(objectUnderTest.getKeyConfig(someOtherKeyConfig).getKeyId(), someOtherKeyConfig); + } + + @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); + + assertThrows(IllegalStateException.class, this::objectUnderTest); + } + + @Test + public void shouldThrowWhenKeyConfigContainsDelimiterCharacter() { + String invalidSubsection = "foo" + KEY_DELIMITER + "bar"; + config.setBoolean(CONF_KEY_SECTION, invalidSubsection, CURRENT_CONFIG_LABEL, false); + + IllegalStateException illegalStateException = + assertThrows(IllegalStateException.class, this::objectUnderTest); + + assertEquals( + illegalStateException.getMessage(), + String.format( + "Configuration error. %s.%s should not contain '%s'", + CONF_KEY_SECTION, invalidSubsection, KEY_DELIMITER)); + } + + @Test + public void shouldThrowWhenMoreThanOneKeyConfigIsSetAsCurrent() { + config.setBoolean(CONF_KEY_SECTION, "someKeyConfig", CURRENT_CONFIG_LABEL, true); + config.setBoolean(CONF_KEY_SECTION, "someOtherKeyConfig", CURRENT_CONFIG_LABEL, true); + + 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 new file mode 100644 index 0000000..3e31d6b --- /dev/null +++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java
@@ -0,0 +1,207 @@ +// 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.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_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; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThrows; + +import com.google.gerrit.extensions.client.AuthType; +import com.google.gerrit.httpd.CanonicalWebUrl; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +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 = 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() { + @Override + protected void configure() { + bind(String.class) + .annotatedWith(com.google.gerrit.server.config.CanonicalWebUrl.class) + .toProvider(Providers.of(null)); + } + }) + .getInstance(CanonicalWebUrl.class); + } + + @Test + 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); + } + + @Test + public void shouldEncryptWithKeyId() throws IOException { + assertEquals(takeKeyId(objectUnderTest().encrypt("someToken")), VERSION1_KEY_ID); + } + + @Test + public void shouldReturnAPrefixedBase64EncodedEncryptedString() throws IOException { + String someOauthToken = "someToken"; + List<String> keyAndMaterial = splitKeyIdFromMaterial(objectUnderTest().encrypt(someOauthToken)); + String keyId = keyAndMaterial.get(0); + String material = keyAndMaterial.get(1); + + assertEquals(keyId, VERSION1_KEY_ID); + assertNotEquals( + Base64.getDecoder().decode(material), someOauthToken.getBytes(StandardCharsets.UTF_8)); + } + + @Test + public void shouldStillBeAbleToDecryptATokenEncryptedWithANonCurrentKey() throws IOException { + String someToken = "someToken"; + String encryptedWithV1 = objectUnderTest().encrypt(someToken); + + config.setBoolean(CONF_KEY_SECTION, VERSION1_KEY_ID, CURRENT_CONFIG_LABEL, false); + config.setBoolean(CONF_KEY_SECTION, VERSION2_KEY_ID, CURRENT_CONFIG_LABEL, true); + + assertEquals(objectUnderTest().decrypt(encryptedWithV1), someToken); + } + + @Test + public void shouldPassThroughWhenDecryptingPlainTextStrings() throws IOException { + String somePlainTextToken = "someToken"; + assertEquals(objectUnderTest().decrypt(somePlainTextToken), somePlainTextToken); + } + + @Test + public void shouldThrowWhenDecryptingATokenEncryptedANoLongerAvailableKey() { + CipherException cipherException = + assertThrows( + CipherException.class, () -> objectUnderTest().decrypt("non-existing-key:foobar")); + + assertEquals( + cipherException.getCause().getMessage(), + "Could not find key-id 'non-existing-key' in configuration"); + } + + @Test + public void shouldThrowWhenCipherAlgorithmIsNotValid() { + config.setString( + CONF_KEY_SECTION, VERSION1_KEY_ID, CIPHER_ALGO_CONFIG_LABEL, "Invalid cipher algorithm"); + + assertThrows(CipherException.class, () -> objectUnderTest().encrypt("some token")); + } + + @Test + public void shouldThrowWhenKeyAlgorithmIsNotValid() { + config.setString( + CONF_KEY_SECTION, VERSION1_KEY_ID, SECRET_KEY_CONFIG_LABEL, "Invalid Key algorithm"); + + assertThrows(CipherException.class, () -> objectUnderTest().encrypt("some token")); + } + + @Test + public void shouldThrowWhenPasswordCouldNotBeRead() { + config.setString( + CONF_KEY_SECTION, VERSION1_KEY_ID, PASSWORD_DEVICE_CONFIG_LABEL, "/some/unexisting/file"); + + assertThrows(IOException.class, this::objectUnderTest); + } + + @Test + public void shouldThrowWhenDecryptingANonBase64String() { + assertThrows( + IOException.class, () -> objectUnderTest().decrypt("current:some non-base64 string")); + } + + private static String takeKeyId(String base64EncryptedString) { + return splitKeyIdFromMaterial(base64EncryptedString).get(0); + } + + private OAuthTokenCipher objectUnderTest() throws IOException { + 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/pom.xml b/github-plugin/pom.xml index fed554e..ed54bf3 100644 --- a/github-plugin/pom.xml +++ b/github-plugin/pom.xml
@@ -20,7 +20,7 @@ <parent> <artifactId>github-parent</artifactId> <groupId>com.googlesource.gerrit.plugins.github</groupId> - <version>3.5.0.1</version> + <version>3.6.0-rc1</version> </parent> <artifactId>github-plugin</artifactId> @@ -101,12 +101,6 @@ <scope>provided</scope> </dependency> <dependency> - <groupId>junit</groupId> - <artifactId>junit</artifactId> - <version>4.8.1</version> - <scope>test</scope> - </dependency> - <dependency> <groupId>${project.groupId}</groupId> <artifactId>github-oauth</artifactId> <version>${project.version}</version> @@ -160,14 +154,8 @@ </dependency> <dependency> <groupId>org.apache.velocity</groupId> - <artifactId>velocity</artifactId> - <version>1.7</version> - </dependency> - <dependency> - <groupId>org.jukito</groupId> - <artifactId>jukito</artifactId> - <version>1.2</version> - <scope>test</scope> + <artifactId>velocity-engine-core</artifactId> + <version>2.3</version> </dependency> </dependencies> </project>
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java index fe9cd2f..010660e 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java
@@ -18,7 +18,6 @@ import com.google.gerrit.entities.Account; import com.google.gerrit.httpd.CanonicalWebUrl; import com.google.gerrit.server.config.AllProjectsNameProvider; -import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; @@ -75,10 +74,9 @@ @GerritServerConfig Config config, final SitePaths site, AllProjectsNameProvider allProjectsNameProvider, - CanonicalWebUrl canonicalWebUrl, - AuthConfig authConfig) + CanonicalWebUrl canonicalWebUrl) throws MalformedURLException { - super(config, canonicalWebUrl, authConfig); + super(config, canonicalWebUrl); String[] wizardFlows = config.getStringList(CONF_SECTION, null, CONF_WIZARD_FLOW); for (String fromTo : wizardFlows) { boolean redirect = fromTo.indexOf(FROM_TO_REDIRECT_SEPARATOR) > 0;
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/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java index 4fc1533..b025737 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubOAuthFilter.java
@@ -26,6 +26,7 @@ import com.googlesource.gerrit.plugins.github.oauth.IdentifiedUserGitHubLoginProvider; import com.googlesource.gerrit.plugins.github.oauth.OAuthFilter; import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken; +import com.googlesource.gerrit.plugins.github.oauth.OAuthTokenCipher; import com.googlesource.gerrit.plugins.github.oauth.OAuthWebFilter; import com.googlesource.gerrit.plugins.github.oauth.ScopedProvider; import java.io.IOException; @@ -47,15 +48,18 @@ private final ScopedProvider<GitHubLogin> loginProvider; private final Provider<CurrentUser> userProvider; private final AccountCache accountCache; + private final OAuthTokenCipher oAuthTokenCipher; @Inject public GitHubOAuthFilter( ScopedProvider<GitHubLogin> loginProvider, Provider<CurrentUser> userProvider, - AccountCache accountCache) { + AccountCache accountCache, + OAuthTokenCipher oAuthTokenCipher) { this.loginProvider = loginProvider; this.userProvider = userProvider; this.accountCache = accountCache; + this.oAuthTokenCipher = oAuthTokenCipher; } @Override @@ -78,7 +82,8 @@ .get() .substring( ExternalId.SCHEME_EXTERNAL.length() + OAuthWebFilter.GITHUB_EXT_ID.length() + 1); - hubLogin.login(new AccessToken(oauthToken)); + String decryptedToken = oAuthTokenCipher.decrypt(oauthToken); + hubLogin.login(new AccessToken(decryptedToken)); } chain.doFilter(request, response);
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/PullRequestCreateChange.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/PullRequestCreateChange.java index 9a16329..3d0ec26 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/PullRequestCreateChange.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/PullRequestCreateChange.java
@@ -43,6 +43,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import java.io.IOException; +import java.time.Instant; import java.util.Collections; import java.util.List; import org.eclipse.jgit.lib.ObjectId; @@ -100,7 +101,9 @@ RestApiException { try (BatchUpdate bu = updateFactory.create( - project.getNameKey(), userFactory.create(pullRequestOwner), TimeUtil.nowTs())) { + project.getNameKey(), + userFactory.create(pullRequestOwner), + Instant.now())) { return internalAddCommitToChange( bu, @@ -254,7 +257,7 @@ Change.id(sequences.nextChangeId()), pullRequestOwner, BranchNameKey.create(project, destRef.getName()), - TimeUtil.nowTs()); + Instant.now()); if (topic != null) { change.setTopic(topic); }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java index 01fe742..1bdce3c 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/notification/WebhookServlet.java
@@ -53,7 +53,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.codec.DecoderException; import org.apache.commons.codec.binary.Hex; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory;
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java index 4970e1d..87fe39d 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityModel.java
@@ -43,7 +43,7 @@ return context.put(key, value); } - public Object remove(Object key) { + public Object remove(String key) { return context.remove(key); } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java index 3e60433..144e77a 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/PluginVelocityRuntimeProvider.java
@@ -25,7 +25,6 @@ import java.util.Properties; import org.apache.velocity.runtime.RuntimeConstants; import org.apache.velocity.runtime.RuntimeInstance; -import org.apache.velocity.runtime.log.Log4JLogChute; import org.apache.velocity.runtime.resource.loader.ClasspathResourceLoader; import org.apache.velocity.runtime.resource.loader.JarResourceLoader; @@ -52,7 +51,6 @@ Properties p = new Properties(); p.setProperty(RuntimeConstants.VM_PERM_INLINE_LOCAL, "true"); - p.setProperty(RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS, Log4JLogChute.class.getName()); p.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, "true"); p.setProperty("runtime.log.logsystem.log4j.category", "velocity");
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java index 06a5556..7ea0283 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityStaticServlet.java
@@ -15,21 +15,26 @@ package com.googlesource.gerrit.plugins.github.velocity; import com.google.common.collect.Maps; +import com.google.gerrit.httpd.raw.SiteStaticDirectoryServlet; +import com.google.gerrit.server.plugins.Plugin; +import com.google.gerrit.server.plugins.PluginEntry; import com.google.gerrit.util.http.CacheHeaders; import com.google.gerrit.util.http.RequestUtil; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import com.google.inject.name.Named; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; +import java.io.*; +import java.nio.charset.StandardCharsets; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; +import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.apache.velocity.runtime.RuntimeInstance; @@ -43,25 +48,17 @@ public class VelocityStaticServlet extends HttpServlet { private static final Logger log = LoggerFactory.getLogger(VelocityStaticServlet.class); private static final Map<String, String> MIME_TYPES = Maps.newHashMap(); + private static final String STATIC_PATH_PREFIX = "static/"; static { MIME_TYPES.put("html", "text/html"); MIME_TYPES.put("htm", "text/html"); MIME_TYPES.put("js", "application/x-javascript"); MIME_TYPES.put("css", "text/css"); - MIME_TYPES.put("rtf", "text/rtf"); - MIME_TYPES.put("txt", "text/plain"); - MIME_TYPES.put("text", "text/plain"); - MIME_TYPES.put("pdf", "application/pdf"); - MIME_TYPES.put("jpeg", "image/jpeg"); - MIME_TYPES.put("jpg", "image/jpeg"); - MIME_TYPES.put("gif", "image/gif"); - MIME_TYPES.put("png", "image/png"); - MIME_TYPES.put("tiff", "image/tiff"); - MIME_TYPES.put("tif", "image/tiff"); - MIME_TYPES.put("svg", "image/svg+xml"); } + private static final Set<String> VELOCITY_STATIC_TYPES = Set.of("html", "htm", "js", "css"); + private static String contentType(final String name) { final int dot = name.lastIndexOf('.'); final String ext = 0 < dot ? name.substring(dot + 1) : ""; @@ -69,14 +66,35 @@ return type != null ? type : "application/octet-stream"; } + private byte[] read(PluginEntry pluginEntry) throws IOException { + try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) { + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + IOUtils.copy(raw, out); + out.flush(); + return out.toByteArray(); + } + } + private static byte[] readResource(final Resource p) throws IOException { - try (InputStream in = p.getResourceLoader().getResourceStream(p.getName()); + try (Reader in = + p.getResourceLoader().getResourceReader(p.getName(), StandardCharsets.UTF_8.name()); ByteArrayOutputStream byteOut = new ByteArrayOutputStream()) { IOUtils.copy(in, byteOut); return byteOut.toByteArray(); } } + private byte[] compress(PluginEntry pluginEntry) throws IOException { + try (InputStream raw = plugin.getContentScanner().getInputStream(pluginEntry)) { + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final GZIPOutputStream gz = new GZIPOutputStream(out); + IOUtils.copy(raw, gz); + gz.finish(); + gz.flush(); + return out.toByteArray(); + } + } + private static byte[] compress(final byte[] raw) throws IOException { final ByteArrayOutputStream out = new ByteArrayOutputStream(); final GZIPOutputStream gz = new GZIPOutputStream(out); @@ -87,16 +105,22 @@ } private final RuntimeInstance velocity; + private final SiteStaticDirectoryServlet siteStaticServlet; + private final Plugin plugin; @Inject VelocityStaticServlet( - @Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider) { + @Named("PluginRuntimeInstance") final Provider<RuntimeInstance> velocityRuntimeProvider, + SiteStaticDirectoryServlet siteStaticServlet, + Plugin plugin) { this.velocity = velocityRuntimeProvider.get(); + this.siteStaticServlet = siteStaticServlet; + this.plugin = plugin; } private Resource local(final HttpServletRequest req) { final String name = req.getPathInfo(); - if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) { + if (name.length() < 2 || !name.startsWith("/")) { // Too short to be a valid file name, or doesn't start with // the path info separator like we expected. // @@ -112,6 +136,24 @@ } } + private boolean isVelocityStaticResource(String resourceName) { + final int dot = resourceName.lastIndexOf('.'); + final String ext = 0 < dot ? resourceName.substring(dot + 1) : ""; + return VELOCITY_STATIC_TYPES.contains(ext.toLowerCase()); + } + + private String resourceName(HttpServletRequest req) { + final String name = req.getPathInfo(); + if (name.length() < 2 || !name.startsWith("/") || isUnreasonableName(name)) { + // Too short to be a valid file name, or doesn't start with + // the path info separator like we expected. + // + return null; + } + + return name.substring(1); + } + private static boolean isUnreasonableName(String name) { if (name.charAt(name.length() - 1) == '/') return true; // no suffix if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths @@ -131,29 +173,68 @@ @Override protected void doGet(final HttpServletRequest req, final HttpServletResponse rsp) - throws IOException { - final Resource p = local(req); - if (p == null) { + throws IOException, ServletException { + String resourceName = resourceName(req); + if (isUnreasonableName(resourceName) || !resourceName.startsWith(STATIC_PATH_PREFIX)) { CacheHeaders.setNotCacheable(rsp); rsp.setStatus(HttpServletResponse.SC_NOT_FOUND); return; } - final String type = contentType(p.getName()); - final byte[] tosend; - if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { - rsp.setHeader("Content-Encoding", "gzip"); - tosend = compress(readResource(p)); - } else { - tosend = readResource(p); + if (isVelocityStaticResource(resourceName)) { + final Resource p = local(req); + if (p == null) { + CacheHeaders.setNotCacheable(rsp); + rsp.setStatus(HttpServletResponse.SC_NOT_FOUND); + return; + } + + final String type = contentType(p.getName()); + final byte[] tosend; + if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { + rsp.setHeader("Content-Encoding", "gzip"); + tosend = compress(readResource(p)); + } else { + tosend = readResource(p); + } + + CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); + rsp.setDateHeader("Last-Modified", p.getLastModified()); + rsp.setContentType(type); + rsp.setContentLength(tosend.length); + try (OutputStream out = rsp.getOutputStream()) { + out.write(tosend); + } } - CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); - rsp.setDateHeader("Last-Modified", p.getLastModified()); - rsp.setContentType(type); - rsp.setContentLength(tosend.length); - try (OutputStream out = rsp.getOutputStream()) { - out.write(tosend); + Optional<PluginEntry> jarResource = plugin.getContentScanner().getEntry(resourceName); + if (jarResource.isPresent()) { + final String type = contentType(resourceName); + final byte[] tosend; + if (!type.equals("application/x-javascript") && RequestUtil.acceptsGzipEncoding(req)) { + rsp.setHeader("Content-Encoding", "gzip"); + tosend = compress(jarResource.get()); + } else { + tosend = read(jarResource.get()); + } + + CacheHeaders.setCacheable(req, rsp, 12, TimeUnit.HOURS); + rsp.setContentType(type); + rsp.setContentLength(tosend.length); + try (OutputStream out = rsp.getOutputStream()) { + out.write(tosend); + } + } else { + + HttpServletRequestWrapper mappedReq = + new HttpServletRequestWrapper(req) { + + @Override + public String getPathInfo() { + return super.getPathInfo().substring(STATIC_PATH_PREFIX.length()); + } + }; + siteStaticServlet.service(mappedReq, rsp); } } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java index 23edcbc..fc578af 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java
@@ -51,7 +51,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang3.StringUtils; import org.eclipse.jgit.errors.ConfigInvalidException; import org.kohsuke.github.GHKey; import org.kohsuke.github.GHMyself;
diff --git a/github-plugin/src/main/resources/Documentation/config.md b/github-plugin/src/main/resources/Documentation/config.md index e8fc5fa..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 @@ -83,4 +93,57 @@ * s, sec, second, seconds * m, min, minute, minutes * h, hr, hour, hours - Default value: 30 seconds \ No newline at end of file + Default value: 30 seconds + +Key Configuration +------------- + +Since this plugin obtains credentials from Github and persists them in Gerrit, +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.\ +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.\ +Default: 16 + +github-key.<key-id>.cipherAlgorithm +: The algorithm to be used for encryption/decryption. 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.\ +Default: AES/ECB/PKCS5Padding + +github-key.<key-id>.secretKeyAlgorithm +: 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.\ +Default: AES + +github-key.<key-id>.current +: Whether this configuration is the current one, and it should be used to +encrypt new Github credentials. Note that _exactly_ one github-key configuration +must be set to `current`, otherwise an error exception will be thrown.\ +Default: false + +As you can observe, in order to support key rotations, multiple `github-key` +can be specified in configuration. credentials encrypted with a `<key-id>` key +can still be decrypted as long as the `github-key.<key-id>` stanza is available +in the configuration. New credentials will always be encrypted with +the `current` `<key-id>`. + +*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. + +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.
diff --git a/github-plugin/src/main/resources/static/scope.html b/github-plugin/src/main/resources/static/scope.html index 5b46f4c..113b5b5 100644 --- a/github-plugin/src/main/resources/static/scope.html +++ b/github-plugin/src/main/resources/static/scope.html
@@ -55,7 +55,7 @@ #set ( $scopeItems = $config.scopes.get($scope) ) #foreach ( $scopeItem in $scopeItems ) $scopeItem.description - #if ( $velocityCount < $scopeItems.size()) + #if ( $foreach.count < $scopeItems.size()) , #end #end
diff --git a/pom.xml b/pom.xml index 72d4915..34b1018 100644 --- a/pom.xml +++ b/pom.xml
@@ -18,7 +18,7 @@ <modelVersion>4.0.0</modelVersion> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.5.0.1</version> + <version>3.6.0-rc1</version> <name>Gerrit Code Review - GitHub integration</name> <url>http://www.gerritforge.com</url> <packaging>pom</packaging> @@ -298,8 +298,18 @@ <dependency> <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> - <version>1.18.16</version> + <version>1.18.22</version> <scope>provided</scope> </dependency> </dependencies> + <dependencyManagement> + <dependencies> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <version>4.13.2</version> + <scope>test</scope> + </dependency> + </dependencies> + </dependencyManagement> </project>