Skip to content

Commit 1d1c224

Browse files
authored
Clientcore keycredpolicy https (Azure#45184)
* Add HTTPS check to KeyCredentialPolicy, fix exception type for OAuthBearerTokenAuthenticationPolicy. * add test for HTTPS check * fixup docs to include @throws * spotless * fix test, add exclusion * remove extra method since it didn't help my doc situation anyway. * remove unused imports
1 parent 7813990 commit 1d1c224

File tree

4 files changed

+46
-1
lines changed

4 files changed

+46
-1
lines changed

sdk/clientcore/core/spotbugs-exclude.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
<Class name="io.clientcore.core.http.models.HttpRequest" />
2020
<Class name="io.clientcore.core.http.pipeline.HttpPipelineBuilder" />
2121
<Class name="io.clientcore.core.http.pipeline.HttpRetryPolicy" />
22+
<Class name="io.clientcore.core.http.pipeline.KeyCredentialPolicy" />
2223
<Class name="io.clientcore.core.http.pipeline.OAuthBearerTokenAuthenticationPolicy" />
2324
<Class name="io.clientcore.core.implementation.GenericParameterizedType" />
2425
<Class name="io.clientcore.core.implementation.ReflectionUtilsMethodHandle" />

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/KeyCredentialPolicy.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,16 @@ private static HttpHeaderName validateName(String name) {
7474
this.prefix = prefix != null ? prefix.trim() : null;
7575
}
7676

77+
/**
78+
* {@inheritDoc}
79+
* @throws IllegalStateException If the request is not using {@code HTTPS}.
80+
*/
7781
@Override
7882
public Response<BinaryData> process(HttpRequest httpRequest, HttpPipelineNextPolicy next) {
83+
if (!"https".equals(httpRequest.getUri().getScheme())) {
84+
throw LOGGER.logThrowableAsError(
85+
new IllegalStateException("Key credentials require HTTPS to prevent leaking the key."));
86+
}
7987
setCredential(httpRequest.getHeaders());
8088
return next.process();
8189
}

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/OAuthBearerTokenAuthenticationPolicy.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,15 @@ public void authorizeRequest(HttpRequest httpRequest, OAuthTokenRequestContext c
6464
httpRequest.getHeaders().set(HttpHeaderName.AUTHORIZATION, BEARER + " " + token.getToken());
6565
}
6666

67+
/**
68+
* {@inheritDoc}
69+
* @throws IllegalStateException If the request is not using {@code HTTPS}.
70+
*/
6771
@Override
6872
public Response<BinaryData> process(HttpRequest httpRequest, HttpPipelineNextPolicy next) {
6973
if (!"https".equals(httpRequest.getUri().getScheme())) {
7074
throw LOGGER.logThrowableAsError(
71-
new RuntimeException("Token credentials require a URL using the HTTPS protocol scheme"));
75+
new IllegalStateException("Token credentials require a URL using the HTTPS protocol scheme"));
7276
}
7377

7478
HttpPipelineNextPolicy nextPolicy = next.copy();

sdk/clientcore/core/src/test/java/io/clientcore/core/http/pipeline/KeyCredentialPolicyTests.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,21 @@
44
package io.clientcore.core.http.pipeline;
55

66
import io.clientcore.core.credentials.KeyCredential;
7+
import io.clientcore.core.http.client.HttpClient;
78
import io.clientcore.core.http.models.HttpHeaderName;
89
import io.clientcore.core.http.models.HttpHeaders;
10+
import io.clientcore.core.http.models.HttpMethod;
11+
import io.clientcore.core.http.models.HttpRequest;
12+
import io.clientcore.core.http.models.Response;
13+
import io.clientcore.core.models.binarydata.BinaryData;
914
import org.junit.jupiter.params.ParameterizedTest;
1015
import org.junit.jupiter.params.provider.Arguments;
1116
import org.junit.jupiter.params.provider.MethodSource;
1217

1318
import java.util.stream.Stream;
1419

1520
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.fail;
1622

1723
public class KeyCredentialPolicyTests {
1824
@ParameterizedTest
@@ -25,6 +31,28 @@ public void setCredential(KeyCredentialPolicy policy, String expectedHeader) {
2531
assertEquals(expectedHeader, headers.getValue(HttpHeaderName.AUTHORIZATION));
2632
}
2733

34+
@ParameterizedTest
35+
@MethodSource("validateSchemesSupplier")
36+
public void validateSchemes(String url, boolean shouldPass) {
37+
KeyCredential credential = new KeyCredential("fakeKeyPlaceholder");
38+
KeyCredentialPolicy policy = new KeyCredentialPolicy(HttpHeaderName.AUTHORIZATION.toString(), credential, null);
39+
HttpPipelinePolicy mockReturnPolicy = (HttpRequest request,
40+
HttpPipelineNextPolicy next) -> new Response<>(request, 200, null, BinaryData.empty());
41+
HttpClient client = (request) -> {
42+
return new Response<>(request, 200, null, BinaryData.empty());
43+
};
44+
HttpPipeline pipeline
45+
= new HttpPipelineBuilder().addPolicy(policy).addPolicy(mockReturnPolicy).httpClient(client).build();
46+
HttpRequest request = new HttpRequest().setMethod(HttpMethod.GET).setUri(url);
47+
try {
48+
pipeline.send(request);
49+
} catch (IllegalStateException e) {
50+
if (shouldPass) {
51+
fail("Expected request to pass but it failed with: " + e.getMessage());
52+
}
53+
}
54+
}
55+
2856
private static Stream<Arguments> setCredentialSupplier() {
2957
String fakeKey = "fakeKeyPlaceholder";
3058
KeyCredential credential = new KeyCredential(fakeKey);
@@ -36,4 +64,8 @@ private static Stream<Arguments> setCredentialSupplier() {
3664
Arguments.of(new KeyCredentialPolicy(HttpHeaderName.AUTHORIZATION.toString(), credential, "Bearer "),
3765
"Bearer " + fakeKey));
3866
}
67+
68+
private static Stream<Arguments> validateSchemesSupplier() {
69+
return Stream.of(Arguments.of("http://localhost", false), Arguments.of("https://localhost", true));
70+
}
3971
}

0 commit comments

Comments
 (0)