Skip to content

Commit ced93f2

Browse files
geminicaprogramsgeminicaprograms
committed
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
1 parent 93a9b50 commit ced93f2

File tree

7 files changed

+454
-88
lines changed

7 files changed

+454
-88
lines changed

‎github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java‎

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
package com.googlesource.gerrit.plugins.github.oauth;
1515

16+
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
17+
1618
import com.google.common.base.CharMatcher;
1719
import com.google.common.base.MoreObjects;
1820
import com.google.common.base.Preconditions;
@@ -38,13 +40,9 @@
3840
import javax.servlet.http.HttpServletRequest;
3941
import lombok.Getter;
4042
import org.eclipse.jgit.lib.Config;
41-
import org.slf4j.Logger;
42-
import org.slf4j.LoggerFactory;
4343

4444
@Singleton
4545
public class GitHubOAuthConfig {
46-
private static Logger logger = LoggerFactory.getLogger(GitHubOAuthConfig.class);
47-
4846
private final Config config;
4947
private final CanonicalWebUrl canonicalWebUrl;
5048

@@ -138,25 +136,16 @@ protected GitHubOAuthConfig(@GerritServerConfig Config config, CanonicalWebUrl c
138136
config.getSubsections(CONF_KEY_SECTION).stream()
139137
.map(KeyConfig::new)
140138
.collect(Collectors.toMap(KeyConfig::getKeyId, Function.identity()));
141-
142-
if (configuredKeyConfig.isEmpty()) {
143-
logger.warn(
144-
"No configured '{}' sections found. Default configuration should NOT be used in production code.",
145-
CONF_KEY_SECTION);
146-
currentKeyConfig = new KeyConfig();
147-
keyConfigMap = Map.of(currentKeyConfig.getKeyId(), currentKeyConfig);
148-
} else {
149-
keyConfigMap = configuredKeyConfig;
150-
List<KeyConfig> currentKeyConfigs =
151-
keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList());
152-
if (currentKeyConfigs.size() != 1) {
153-
throw new IllegalStateException(
154-
String.format(
155-
"Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
156-
CONF_KEY_SECTION, currentKeyConfigs.size()));
157-
}
158-
currentKeyConfig = currentKeyConfigs.get(0);
139+
keyConfigMap = configuredKeyConfig;
140+
List<KeyConfig> currentKeyConfigs =
141+
keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList());
142+
if (currentKeyConfigs.size() != 1) {
143+
throw new IllegalStateException(
144+
String.format(
145+
"Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
146+
CONF_KEY_SECTION, currentKeyConfigs.size()));
159147
}
148+
currentKeyConfig = currentKeyConfigs.get(0);
160149
}
161150

162151
public String getOAuthFinalRedirectUrl(HttpServletRequest req) {
@@ -220,7 +209,6 @@ public KeyConfig getKeyConfig(String subsection) {
220209

221210
public class KeyConfig {
222211

223-
public static final String PASSWORD_DEVICE_DEFAULT = "/dev/zero";
224212
public static final int PASSWORD_LENGTH_DEFAULT = 16;
225213
public static final String CIPHER_ALGORITHM_DEFAULT = "AES/ECB/PKCS5Padding";
226214
public static final String SECRET_KEY_ALGORITHM_DEFAULT = "AES";
@@ -251,11 +239,7 @@ public class KeyConfig {
251239
CONF_KEY_SECTION, keyId, KEY_DELIMITER));
252240
}
253241

254-
this.passwordDevice =
255-
trimTrailingSlash(
256-
MoreObjects.firstNonNull(
257-
config.getString(CONF_KEY_SECTION, keyId, PASSWORD_DEVICE_CONFIG_LABEL),
258-
PASSWORD_DEVICE_DEFAULT));
242+
this.passwordDevice = trimTrailingSlash(getPasswordDeviceOrThrow(config, keyId));
259243
this.passwordLength =
260244
config.getInt(
261245
CONF_KEY_SECTION, keyId, PASSWORD_LENGTH_CONFIG_LABEL, PASSWORD_LENGTH_DEFAULT);
@@ -274,15 +258,6 @@ public class KeyConfig {
274258
this.keyId = keyId;
275259
}
276260

277-
private KeyConfig() {
278-
passwordDevice = PASSWORD_DEVICE_DEFAULT;
279-
passwordLength = PASSWORD_LENGTH_DEFAULT;
280-
isCurrent = true;
281-
cipherAlgorithm = CIPHER_ALGORITHM_DEFAULT;
282-
keyId = KEY_ID_DEFAULT;
283-
secretKeyAlgorithm = SECRET_KEY_ALGORITHM_DEFAULT;
284-
}
285-
286261
public byte[] readPassword() throws IOException {
287262
Path devicePath = Paths.get(passwordDevice);
288263
try (FileInputStream in = new FileInputStream(devicePath.toFile())) {
@@ -311,4 +286,22 @@ public String getKeyId() {
311286
return keyId;
312287
}
313288
}
289+
290+
/**
291+
* Method returns the password device value for a given {@code keyId}.
292+
*
293+
* @throws {@link IllegalStateException} when password device is not configured for {@code keyId}
294+
*/
295+
private static String getPasswordDeviceOrThrow(Config config, String keyId) {
296+
String passwordDevice =
297+
config.getString(CONF_KEY_SECTION, keyId, KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL);
298+
if (Strings.isNullOrEmpty(passwordDevice)) {
299+
throw new IllegalStateException(
300+
String.format(
301+
"Configuration error. Missing %s.%s for key-id '%s'",
302+
CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, keyId));
303+
}
304+
305+
return passwordDevice;
306+
}
314307
}

‎github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// Copyright (C) 2022 The Android Open Source Project
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.googlesource.gerrit.plugins.github.oauth;
16+
17+
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT;
18+
import static java.util.Objects.requireNonNull;
19+
20+
import java.io.File;
21+
import java.io.IOException;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
24+
import java.security.SecureRandom;
25+
import org.slf4j.Logger;
26+
import org.slf4j.LoggerFactory;
27+
28+
public class PasswordGenerator {
29+
private static final Logger logger = LoggerFactory.getLogger(PasswordGenerator.class);
30+
public static final String DEFAULT_PASSWORD_FILE = "default.key";
31+
32+
/**
33+
* Generates default password and stores under given {@code Path}. Note that if password already
34+
* exists it is not regenerated.
35+
*
36+
* @param passwordFilePath path that should contain the default password; cannot be {@code null}
37+
* @throws {@link IllegalStateException} when file denoted by given {@code Path} is a directory,
38+
* cannot be read, has invalid length or doesn't exist and cannot be created
39+
* @return {@code true} if password was generated, {@code false} if it already exists
40+
*/
41+
public boolean generate(Path passwordFilePath) {
42+
requireNonNull(passwordFilePath);
43+
44+
File passwordFile = passwordFilePath.toFile();
45+
46+
if (passwordFile.isDirectory()) {
47+
throw logErrorAndCreateRuntimeException(
48+
"'%s' is directory whilst a regular file was expected.", passwordFilePath);
49+
}
50+
51+
if (passwordFile.isFile()) {
52+
if (!passwordFile.canRead()) {
53+
throw logErrorAndCreateRuntimeException(
54+
"'%s' password file exists, but cannot be read.", passwordFilePath);
55+
}
56+
57+
long length = passwordFile.length();
58+
if (length != PASSWORD_LENGTH_DEFAULT) {
59+
throw logErrorAndCreateRuntimeException(
60+
"'%s' password file exists but has an invalid length of %d bytes. The expected length is %d bytes.",
61+
passwordFilePath, length, PASSWORD_LENGTH_DEFAULT);
62+
}
63+
return false;
64+
}
65+
66+
byte[] token = generateToken();
67+
try {
68+
Files.write(passwordFilePath, token);
69+
logger.info("Password was stored in {} file", passwordFilePath);
70+
return true;
71+
} catch (IOException e) {
72+
throw logErrorAndCreateRuntimeException(e, "Password generation has failed");
73+
}
74+
}
75+
76+
private byte[] generateToken() {
77+
SecureRandom random = new SecureRandom();
78+
byte[] token = new byte[PASSWORD_LENGTH_DEFAULT];
79+
random.nextBytes(token);
80+
return token;
81+
}
82+
83+
private IllegalStateException logErrorAndCreateRuntimeException(
84+
String msg, Object... parameters) {
85+
return logErrorAndCreateRuntimeException(null, msg, parameters);
86+
}
87+
88+
private IllegalStateException logErrorAndCreateRuntimeException(
89+
Exception e, String msg, Object... parameters) {
90+
String log = String.format(msg, parameters);
91+
if (e != null) {
92+
logger.error(log, e);
93+
return new IllegalStateException(log, e);
94+
}
95+
96+
logger.error(log);
97+
return new IllegalStateException(log);
98+
}
99+
}

‎github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@
1515

1616
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_KEY_SECTION;
1717
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_SECTION;
18-
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGORITHM_DEFAULT;
1918
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL;
2019
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL;
2120
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_DELIMITER;
22-
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT;
23-
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_ALGORITHM_DEFAULT;
21+
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL;
2422
import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL;
2523
import static org.junit.Assert.assertEquals;
2624
import static org.junit.Assert.assertThrows;
@@ -38,6 +36,7 @@ public class GitHubOAuthConfigTest {
3836

3937
CanonicalWebUrl canonicalWebUrl;
4038
Config config;
39+
private static final String testPasswordDevice = "/dev/zero";
4140

4241
@Before
4342
public void setUp() {
@@ -60,29 +59,14 @@ protected void configure() {
6059
.getInstance(CanonicalWebUrl.class);
6160
}
6261

63-
@Test
64-
public void shouldDefaultKeyConfigWhenNoSpecificConfigurationIsSet() {
65-
GitHubOAuthConfig objectUnderTest = objectUnderTest();
66-
assertEquals(objectUnderTest.getCurrentKeyConfig().isCurrent(), true);
67-
assertEquals(
68-
objectUnderTest.getCurrentKeyConfig().getCipherAlgorithm(), CIPHER_ALGORITHM_DEFAULT);
69-
assertEquals(
70-
objectUnderTest.getCurrentKeyConfig().getSecretKeyAlgorithm(),
71-
SECRET_KEY_ALGORITHM_DEFAULT);
72-
assertEquals(objectUnderTest.getCurrentKeyConfig().getKeyId(), KEY_ID_DEFAULT);
73-
}
74-
75-
@Test
76-
public void shouldDBeAbleToLookupDefaultKeyWhenNoSpecificConfigurationIsSet() {
77-
assertEquals(objectUnderTest().getKeyConfig(KEY_ID_DEFAULT).isCurrent(), true);
78-
}
79-
8062
@Test
8163
public void shouldReadASpecificKeyConfig() {
8264
String keySubsection = "someKeyConfig";
8365
String cipherAlgorithm = "AES/CFB8/NoPadding";
8466
String secretKeyAlgorithm = "DES";
8567
config.setBoolean(CONF_KEY_SECTION, keySubsection, CURRENT_CONFIG_LABEL, true);
68+
config.setString(
69+
CONF_KEY_SECTION, keySubsection, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
8670
config.setString(CONF_KEY_SECTION, keySubsection, CIPHER_ALGO_CONFIG_LABEL, cipherAlgorithm);
8771
config.setString(CONF_KEY_SECTION, keySubsection, SECRET_KEY_CONFIG_LABEL, secretKeyAlgorithm);
8872

@@ -96,25 +80,47 @@ public void shouldReadASpecificKeyConfig() {
9680

9781
@Test
9882
public void shouldReturnTheExpectedKeyConfigAsCurrent() {
99-
config.setBoolean(CONF_KEY_SECTION, "currentKeyConfig", CURRENT_CONFIG_LABEL, true);
100-
config.setBoolean(CONF_KEY_SECTION, "someOtherKeyConfig", CURRENT_CONFIG_LABEL, false);
83+
String currentKeyConfig = "currentKeyConfig";
84+
String someOtherKeyConfig = "someOtherKeyConfig";
85+
config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true);
86+
config.setString(
87+
CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
88+
config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false);
89+
config.setString(
90+
CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
10191

102-
assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), "currentKeyConfig");
92+
assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), currentKeyConfig);
10393
}
10494

10595
@Test
10696
public void shouldReadMultipleKeyConfigs() {
10797
String currentKeyConfig = "currentKeyConfig";
10898
String someOtherKeyConfig = "someOtherKeyConfig";
10999
config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true);
100+
config.setString(
101+
CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
110102
config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false);
103+
config.setString(
104+
CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice);
111105

112106
GitHubOAuthConfig objectUnderTest = objectUnderTest();
113107

114108
assertEquals(objectUnderTest.getKeyConfig(currentKeyConfig).getKeyId(), currentKeyConfig);
115109
assertEquals(objectUnderTest.getKeyConfig(someOtherKeyConfig).getKeyId(), someOtherKeyConfig);
116110
}
117111

112+
@Test
113+
public void shouldThrowWhenNoKeyIdIsConfigured() {
114+
IllegalStateException illegalStateException =
115+
assertThrows(IllegalStateException.class, this::objectUnderTest);
116+
117+
assertEquals(
118+
illegalStateException.getMessage(),
119+
String.format(
120+
"Expected exactly 1 subsection of '%s' to be configured as 'current', %d found",
121+
CONF_KEY_SECTION, 0));
122+
}
123+
118124
@Test
119125
public void shouldThrowWhenNoKeyConfigIsSetAsCurrent() {
120126
config.setBoolean(CONF_KEY_SECTION, "someKeyConfig", CURRENT_CONFIG_LABEL, false);
@@ -145,6 +151,21 @@ public void shouldThrowWhenMoreThanOneKeyConfigIsSetAsCurrent() {
145151
assertThrows(IllegalStateException.class, this::objectUnderTest);
146152
}
147153

154+
@Test
155+
public void shouldThrowWhenKeyIdMissesPasswordDevice() {
156+
String someKeyConfig = "someKeyConfig";
157+
config.setBoolean(CONF_KEY_SECTION, someKeyConfig, CURRENT_CONFIG_LABEL, true);
158+
159+
IllegalStateException illegalStateException =
160+
assertThrows(IllegalStateException.class, this::objectUnderTest);
161+
162+
assertEquals(
163+
String.format(
164+
"Configuration error. Missing %s.%s for key-id '%s'",
165+
CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, someKeyConfig),
166+
illegalStateException.getMessage());
167+
}
168+
148169
private GitHubOAuthConfig objectUnderTest() {
149170
return new GitHubOAuthConfig(config, canonicalWebUrl);
150171
}

0 commit comments

Comments
 (0)