Skip to content

Commit a5b41a6

Browse files
WeltraumschafManuelNeuer
Weltraumschaf
authored and
ManuelNeuer
committed
#36 Use constants to get proxy auth config
Remove duplication of constant names by introducing an enum with the properties names. Also adds some basic docs how it works and a basic validation of the properties w/ error handling. Signed-off-by: Sven Strittmatter <sven.strittmatter@iteratec.com>
1 parent 4ca6ae6 commit a5b41a6

File tree

3 files changed

+245
-18
lines changed

3 files changed

+245
-18
lines changed

‎src/main/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanService.java

+76-11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.springframework.http.HttpHeaders;
2222
import org.springframework.http.HttpMethod;
2323
import org.springframework.http.MediaType;
24+
import org.springframework.http.client.ClientHttpRequestFactory;
2425
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
2526
import org.springframework.http.converter.FormHttpMessageConverter;
2627
import org.springframework.http.converter.ResourceHttpMessageConverter;
@@ -34,6 +35,7 @@
3435
import java.util.List;
3536

3637
final class DefaultImportScanService implements ImportScanService {
38+
private final SystemPropertyFinder properties = new SystemPropertyFinder();
3739
private final String defectDojoUrl;
3840
private final String defectDojoApiKey;
3941

@@ -88,9 +90,9 @@ private ImportScanResponse createFindings(ScanFile scanFile, String endpoint, lo
8890
final var headers = createDefectDojoAuthorizationHeaders();
8991
headers.setContentType(MediaType.MULTIPART_FORM_DATA);
9092
restTemplate.setMessageConverters(List.of(
91-
new FormHttpMessageConverter(),
92-
new ResourceHttpMessageConverter(),
93-
new MappingJackson2HttpMessageConverter())
93+
new FormHttpMessageConverter(),
94+
new ResourceHttpMessageConverter(),
95+
new MappingJackson2HttpMessageConverter())
9496
);
9597

9698
// FIXME: Why do we use a multi value map here? Do we need multiple values for any given key?
@@ -154,22 +156,67 @@ private RestTemplate createRestTemplate() {
154156
return template;
155157
}
156158

157-
private static boolean shouldConfigureProxySettings() {
158-
return System.getProperty("http.proxyUser") != null && System.getProperty("http.proxyPassword") != null;
159+
boolean shouldConfigureProxySettings() {
160+
return properties.hasProperty(ProxyConfigNames.HTTP_PROXY_USER)
161+
&& properties.hasProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD);
159162
}
160163

161-
private static HttpComponentsClientHttpRequestFactory createRequestFactoryWithProxyAuthConfig() {
162-
// Configuring Proxy Authentication explicitly as it isn't done by default for spring rest templates :(
164+
/**
165+
* Configuring proxy authentication explicitly
166+
*
167+
* <p>
168+
* This isn't done by default for spring rest templates.This method expects these four system properties (Java flag
169+
* {@literal -DpropertyName}) to be set:
170+
* </p>
171+
* <ul>
172+
* <li>http.proxyUser</li>
173+
* <li>http.proxyPassword</li>
174+
* <li>http.proxyHost</li>
175+
* <li>http.proxyPort</li>
176+
* </ul>
177+
*
178+
* @return never {@code null}
179+
*/
180+
ClientHttpRequestFactory createRequestFactoryWithProxyAuthConfig() {
181+
if (properties.notHasProperty(ProxyConfigNames.HTTP_PROXY_USER)) {
182+
throw new MissingProxyAuthenticationConfig(ProxyConfigNames.HTTP_PROXY_USER);
183+
}
184+
185+
if (properties.notHasProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD)) {
186+
throw new MissingProxyAuthenticationConfig(ProxyConfigNames.HTTP_PROXY_PASSWORD);
187+
}
188+
189+
if (properties.notHasProperty(ProxyConfigNames.HTTP_PROXY_HOST)) {
190+
throw new MissingProxyAuthenticationConfig(ProxyConfigNames.HTTP_PROXY_HOST);
191+
}
192+
193+
if (properties.notHasProperty(ProxyConfigNames.HTTP_PROXY_PORT)) {
194+
throw new MissingProxyAuthenticationConfig(ProxyConfigNames.HTTP_PROXY_PORT);
195+
}
196+
197+
final var proxyHost = properties.getProperty(ProxyConfigNames.HTTP_PROXY_HOST);
198+
final int proxyPort;
199+
try {
200+
proxyPort = Integer.parseInt(properties.getProperty(ProxyConfigNames.HTTP_PROXY_PORT));
201+
} catch (final NumberFormatException e) {
202+
throw new IllegalArgumentException(
203+
String.format("Given port for proxy authentication configuration (property '%s') is not a valid number! Given value wa '%s'.",
204+
ProxyConfigNames.HTTP_PROXY_PORT.getLiterat(),
205+
System.getProperty("http.proxyPort")),
206+
e);
207+
}
208+
163209
final var credentials = new BasicCredentialsProvider();
164210
credentials.setCredentials(
165-
new AuthScope(System.getProperty("http.proxyHost"), Integer.parseInt(System.getProperty("http.proxyPort"))),
166-
new UsernamePasswordCredentials(System.getProperty("http.proxyUser"), System.getProperty("http.proxyPassword"))
211+
new AuthScope(proxyHost, proxyPort),
212+
new UsernamePasswordCredentials(
213+
properties.getProperty(ProxyConfigNames.HTTP_PROXY_USER),
214+
properties.getProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD))
167215
);
168216

169217
final var clientBuilder = HttpClientBuilder.create();
170-
171218
clientBuilder.useSystemProperties();
172-
clientBuilder.setProxy(new HttpHost(System.getProperty("http.proxyHost"), Integer.parseInt(System.getProperty("http.proxyPort"))));
219+
clientBuilder.setProxy(new HttpHost(proxyHost, proxyPort));
173220
clientBuilder.setDefaultCredentialsProvider(credentials);
174221
clientBuilder.setProxyAuthenticationStrategy(new ProxyAuthenticationStrategy());
175222

@@ -178,5 +225,23 @@ private static HttpComponentsClientHttpRequestFactory createRequestFactoryWithPr
178225
return factory;
179226
}
180227

228+
private static class SystemPropertyFinder {
229+
private boolean hasProperty(@NonNull final ProxyConfigNames name) {
230+
return System.getProperty(name.getLiterat()) != null;
231+
}
232+
233+
private boolean notHasProperty(@NonNull final ProxyConfigNames name) {
234+
return !hasProperty(name);
235+
}
236+
237+
private String getProperty(@NonNull final ProxyConfigNames name) {
238+
return System.getProperty(name.getLiterat());
239+
}
240+
}
181241

242+
final static class MissingProxyAuthenticationConfig extends RuntimeException {
243+
MissingProxyAuthenticationConfig(ProxyConfigNames name) {
244+
super(String.format("Expected System property '%s' not set!", name.getLiterat()));
245+
}
246+
}
182247
}

‎src/main/java/io/securecodebox/persistence/defectdojo/service/ImportScanService.java

+25
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.securecodebox.persistence.defectdojo.config.DefectDojoConfig;
1111
import io.securecodebox.persistence.defectdojo.models.ScanFile;
1212
import lombok.Data;
13+
import lombok.Getter;
1314
import lombok.NonNull;
1415
import org.springframework.util.MultiValueMap;
1516

@@ -46,4 +47,28 @@ class ImportScanResponse {
4647
@JsonProperty("test")
4748
protected long testId;
4849
}
50+
51+
/**
52+
* These properties can be configured by passing them to the running Java process w/ flag {@literal -D}
53+
* <p>
54+
* Example: {@literal java -Dhttp.proxyHost=... -D... -jar ...}
55+
* </p>
56+
* <p>
57+
* <strong>Important</strong>: All four parameters are mandatory. You must set them all
58+
* or none of them!
59+
* </p>
60+
*/
61+
enum ProxyConfigNames {
62+
HTTP_PROXY_HOST("http.proxyHost"),
63+
HTTP_PROXY_PORT("http.proxyPort"),
64+
HTTP_PROXY_USER("http.proxyUser"),
65+
HTTP_PROXY_PASSWORD("http.proxyPassword");
66+
67+
@Getter
68+
private final String literat;
69+
70+
ProxyConfigNames(String literat) {
71+
this.literat = literat;
72+
}
73+
}
4974
}

‎src/test/java/io/securecodebox/persistence/defectdojo/service/DefaultImportScanServiceTest.java

+144-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
package io.securecodebox.persistence.defectdojo.service;
22

33
import io.securecodebox.persistence.defectdojo.config.DefectDojoConfig;
4+
import io.securecodebox.persistence.defectdojo.service.DefaultImportScanService.MissingProxyAuthenticationConfig;
5+
import io.securecodebox.persistence.defectdojo.service.ImportScanService.ProxyConfigNames;
6+
import org.junit.jupiter.api.AfterEach;
7+
import org.junit.jupiter.api.BeforeEach;
48
import org.junit.jupiter.api.Test;
59
import org.springframework.http.HttpHeaders;
610

11+
import java.util.HashMap;
12+
import java.util.Map;
13+
714
import static org.junit.jupiter.api.Assertions.*;
815
import static org.hamcrest.MatcherAssert.assertThat;
916

@@ -12,13 +19,39 @@
1219
*/
1320
class DefaultImportScanServiceTest {
1421
private final DefectDojoConfig config = new DefectDojoConfig(
15-
"url",
16-
"apiKey",
17-
"username",
18-
23,
19-
42L
22+
"url",
23+
"apiKey",
24+
"username",
25+
23,
26+
42L
2027
);
2128
private final DefaultImportScanService sut = new DefaultImportScanService(config);
29+
/**
30+
* Since System.getProperty() is an side effect we need to back up and restore it to isolate test cases.
31+
*/
32+
private final Map<ProxyConfigNames, String> backup = new HashMap<>();
33+
34+
@BeforeEach
35+
void backupSystemProperties() {
36+
backup.clear();
37+
backup.put(ProxyConfigNames.HTTP_PROXY_HOST, System.getProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat()));
38+
backup.put(ProxyConfigNames.HTTP_PROXY_PORT, System.getProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat()));
39+
backup.put(ProxyConfigNames.HTTP_PROXY_USER, System.getProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat()));
40+
backup.put(ProxyConfigNames.HTTP_PROXY_PASSWORD, System.getProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat()));
41+
}
42+
43+
@AfterEach
44+
void restoreSystemProperties() {
45+
for (final var entry : backup.entrySet()) {
46+
final var name = entry.getKey().getLiterat();
47+
48+
if (null == entry.getValue()) {
49+
System.clearProperty(name);
50+
} else {
51+
System.setProperty(name, entry.getValue());
52+
}
53+
}
54+
}
2255

2356
@Test
2457
void constructorShouldThrowExceptionOnNullConfig() {
@@ -31,8 +64,112 @@ void constructorShouldThrowExceptionOnNullConfig() {
3164
void createDefectDojoAuthorizationHeaders_apiKeyFromConfigShouldBePresentAsAuthHEader() {
3265
final var authorizationHeaders = sut.createDefectDojoAuthorizationHeaders();
3366
assertAll(
34-
() -> assertEquals(1, authorizationHeaders.size(), "Expected is exactly one authorization header!"),
35-
() -> assertEquals("Token apiKey", authorizationHeaders.get(HttpHeaders.AUTHORIZATION).get(0))
67+
() -> assertEquals(1, authorizationHeaders.size(), "Expected is exactly one authorization header!"),
68+
() -> assertEquals("Token apiKey", authorizationHeaders.get(HttpHeaders.AUTHORIZATION).get(0))
3669
);
3770
}
71+
72+
@Test
73+
void shouldConfigureProxySettings_falseIfNeitherUserNorPasswordIsSet() {
74+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat());
75+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat());
76+
77+
assertFalse(sut.shouldConfigureProxySettings());
78+
}
79+
80+
@Test
81+
void shouldConfigureProxySettings_falseIfUserSetButPasswordNot() {
82+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
83+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat());
84+
85+
assertFalse(sut.shouldConfigureProxySettings());
86+
}
87+
88+
@Test
89+
void shouldConfigureProxySettings_falseIfPasswordSetButUserNot() {
90+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat());
91+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
92+
93+
assertFalse(sut.shouldConfigureProxySettings());
94+
}
95+
96+
@Test
97+
void shouldConfigureProxySettings_trueIfUserAndPasswordSet() {
98+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
99+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
100+
101+
assertTrue(sut.shouldConfigureProxySettings());
102+
}
103+
104+
@Test
105+
void createRequestFactoryWithProxyAuthConfig_throesExceptionIfUserNotSet() {
106+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat());
107+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
108+
System.setProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat(), "host");
109+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat(), "4242");
110+
111+
final var thrown = assertThrows(
112+
MissingProxyAuthenticationConfig.class,
113+
sut::createRequestFactoryWithProxyAuthConfig);
114+
115+
assertEquals("Expected System property 'http.proxyUser' not set!", thrown.getMessage());
116+
}
117+
118+
@Test
119+
void createRequestFactoryWithProxyAuthConfig_throesExceptionIfPasswordNotSet() {
120+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
121+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat());
122+
System.setProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat(), "host");
123+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat(), "4242");
124+
125+
final var thrown = assertThrows(
126+
MissingProxyAuthenticationConfig.class,
127+
sut::createRequestFactoryWithProxyAuthConfig);
128+
129+
assertEquals("Expected System property 'http.proxyPassword' not set!", thrown.getMessage());
130+
}
131+
132+
@Test
133+
void createRequestFactoryWithProxyAuthConfig_throesExceptionIfHostNotSet() {
134+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
135+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
136+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat());
137+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat(), "4242");
138+
139+
final var thrown = assertThrows(
140+
MissingProxyAuthenticationConfig.class,
141+
sut::createRequestFactoryWithProxyAuthConfig);
142+
143+
assertEquals("Expected System property 'http.proxyHost' not set!", thrown.getMessage());
144+
}
145+
146+
@Test
147+
void createRequestFactoryWithProxyAuthConfig_throesExceptionIfPortNotSet() {
148+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
149+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
150+
System.setProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat(), "host");
151+
System.clearProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat());
152+
153+
final var thrown = assertThrows(
154+
MissingProxyAuthenticationConfig.class,
155+
sut::createRequestFactoryWithProxyAuthConfig);
156+
157+
assertEquals("Expected System property 'http.proxyPort' not set!", thrown.getMessage());
158+
}
159+
160+
@Test
161+
void createRequestFactoryWithProxyAuthConfig_throesExceptionIfPortIsNotInteger() {
162+
System.setProperty(ProxyConfigNames.HTTP_PROXY_USER.getLiterat(), "user");
163+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PASSWORD.getLiterat(), "password");
164+
System.setProperty(ProxyConfigNames.HTTP_PROXY_HOST.getLiterat(), "host");
165+
System.setProperty(ProxyConfigNames.HTTP_PROXY_PORT.getLiterat(), "FUBAR");
166+
167+
final var thrown = assertThrows(
168+
IllegalArgumentException.class,
169+
sut::createRequestFactoryWithProxyAuthConfig);
170+
171+
assertEquals(
172+
"Given port for proxy authentication configuration (property 'http.proxyPort') is not a valid number! Given value wa 'FUBAR'.",
173+
thrown.getMessage());
174+
}
38175
}

0 commit comments

Comments
 (0)