Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[After MSAL 1.3] Pluggable HTTP in identity/msal #7120

Merged
merged 17 commits into from
Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/versioning/external_dependencies.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ com.microsoft.azure:azure-mgmt-keyvault;1.11.1
com.microsoft.azure:azure-mgmt-resources;1.3.0
com.microsoft.azure:azure-mgmt-storage;1.3.0
com.microsoft.azure:azure-storage;8.0.0
com.microsoft.azure:msal4j;0.5.0-preview
com.microsoft.azure:msal4j;1.3.0
io.opentelemetry:opentelemetry-api;0.2.0
io.opentelemetry:opentelemetry-sdk;0.2.0
io.projectreactor:reactor-test;3.3.0.RELEASE
Expand Down
2 changes: 1 addition & 1 deletion sdk/eventhubs/microsoft-azure-eventhubs/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
<dependency>
<groupId>com.microsoft.azure</groupId>
<artifactId>msal4j</artifactId>
<version>0.5.0-preview</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
<version>1.3.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is track 1 event hubs module. Did you intend to update msal version of track 1 module too? This might need service team's validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be only used in tests. And I don't know a way to specify a dependency of 2 different versions across our SDKs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because you are using the script to update the external dependency version and it's updating both track 1 and track 2 dependencies?

<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

package com.microsoft.azure.eventhubs.sendrecv;

import com.microsoft.aad.msal4j.ClientCredentialFactory;
import com.microsoft.aad.msal4j.ClientCredentialParameters;
import com.microsoft.aad.msal4j.ClientSecret;
import com.microsoft.aad.msal4j.ConfidentialClientApplication;
import com.microsoft.aad.msal4j.IAuthenticationResult;
import com.microsoft.azure.eventhubs.AzureActiveDirectoryTokenProvider;
Expand Down Expand Up @@ -61,7 +61,7 @@ public void runSendReceiveWithCustomTokenProvider() throws Exception {
@Override
String tokenGet(final String authority, final String clientId, final String clientSecret, final String audience, final String extra)
throws MalformedURLException, InterruptedException, ExecutionException {
ConfidentialClientApplication app = ConfidentialClientApplication.builder(clientId, new ClientSecret(clientSecret))
ConfidentialClientApplication app = ConfidentialClientApplication.builder(clientId, ClientCredentialFactory.createFromSecret(clientSecret))
.authority(authority)
.build();

Expand Down
15 changes: 14 additions & 1 deletion sdk/identity/azure-identity/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<dependency>
<groupId>com.microsoft.azure</groupId>
<artifactId>msal4j</artifactId>
<version>0.5.0-preview</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
<version>1.3.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
</dependency>
<dependency>
<groupId>com.nimbusds</groupId>
Expand Down Expand Up @@ -81,5 +81,18 @@
<version>3.3.0.RELEASE</version> <!-- {x-version-update;io.projectreactor:reactor-test;external_dependency} -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-core-http-netty</artifactId>
<version>1.2.0</version> <!-- {x-version-update;com.azure:azure-core-http-netty;dependency} -->
<scope>test</scope>
</dependency>
<!-- for file lock tests, ideally should be removed in the future -->
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.8.5</version> <!-- {x-version-update;com.google.code.gson:gson;external_dependency} -->
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.azure.identity;

import com.azure.core.http.HttpPipeline;
import com.azure.core.http.ProxyOptions;
import com.azure.identity.implementation.IdentityClientOptions;

Expand Down Expand Up @@ -55,4 +56,16 @@ public T proxyOptions(ProxyOptions proxyOptions) {
this.identityClientOptions.setProxyOptions(proxyOptions);
return (T) this;
}

/**
* Specifies the HttpPipeline to send all requests. This setting overrides the others.
*
* @param httpPipeline the HttpPipeline to send all requests
* @return itself
*/
@SuppressWarnings("unchecked")
public T httpPipeline(HttpPipeline httpPipeline) {
this.identityClientOptions.setHttpPipeline(httpPipeline);
return (T) this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.identity.implementation;

import com.azure.core.http.HttpHeader;
import com.azure.core.http.HttpHeaders;
import com.azure.core.http.HttpMethod;
import com.azure.core.http.HttpPipeline;
import com.microsoft.aad.msal4j.HttpRequest;
import com.microsoft.aad.msal4j.IHttpClient;
import com.microsoft.aad.msal4j.IHttpResponse;
import reactor.core.publisher.Mono;

import java.util.Collections;
import java.util.stream.Collectors;

/**
* Adapts an HttpPipeline to an instance of IHttpClient in the MSAL4j pipeline.
*/
class HttpPipelineAdapter implements IHttpClient {
private final HttpPipeline httpPipeline;

HttpPipelineAdapter(HttpPipeline httpPipeline) {
this.httpPipeline = httpPipeline;
}

@Override
public IHttpResponse send(HttpRequest httpRequest) throws Exception {
// convert request
com.azure.core.http.HttpRequest request = new com.azure.core.http.HttpRequest(
HttpMethod.valueOf(httpRequest.httpMethod().name()),
httpRequest.url());
if (httpRequest.headers() != null) {
request.setHeaders(new HttpHeaders(httpRequest.headers()));
}
if (httpRequest.body() != null) {
request.setBody(httpRequest.body());
}

return httpPipeline.send(request)
.flatMap(response -> response.getBodyAsString()
.map(body -> {
com.microsoft.aad.msal4j.HttpResponse httpResponse = new com.microsoft.aad.msal4j.HttpResponse()
.body(body)
.statusCode(response.getStatusCode());
httpResponse.headers(response.getHeaders().stream().collect(Collectors.toMap(HttpHeader::getName,
h -> Collections.singletonList(h.getValue()))));
return httpResponse;
})
// if no body
.switchIfEmpty(Mono.defer(() -> {
com.microsoft.aad.msal4j.HttpResponse httpResponse = new com.microsoft.aad.msal4j.HttpResponse()
.statusCode(response.getStatusCode());
httpResponse.headers(response.getHeaders().stream().collect(Collectors.toMap(HttpHeader::getName,
h -> Collections.singletonList(h.getValue()))));
return Mono.just(httpResponse);
})))
.block();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
import com.azure.core.credential.AccessToken;
import com.azure.core.credential.TokenRequestContext;
import com.azure.core.http.ProxyOptions;
import com.azure.core.util.logging.ClientLogger;
import com.azure.core.util.serializer.JacksonAdapter;
import com.azure.core.util.serializer.SerializerAdapter;
import com.azure.core.util.serializer.SerializerEncoding;
import com.azure.core.util.serializer.JacksonAdapter;
import com.azure.core.util.logging.ClientLogger;
import com.azure.identity.DeviceCodeInfo;
import com.azure.identity.implementation.util.CertificateUtil;
import com.microsoft.aad.msal4j.AuthorizationCodeParameters;
Expand Down Expand Up @@ -37,11 +37,6 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.UnrecoverableKeyException;
import java.security.cert.CertificateException;
import java.time.Duration;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
Expand Down Expand Up @@ -87,6 +82,10 @@ public class IdentityClient {
} else {
String authorityUrl = options.getAuthorityHost().replaceAll("/+$", "") + "/organizations/" + tenantId;
PublicClientApplication.Builder publicClientApplicationBuilder = PublicClientApplication.builder(clientId);
if (options.getHttpPipeline() != null) {
publicClientApplicationBuilder = publicClientApplicationBuilder
.httpClient(new HttpPipelineAdapter(options.getHttpPipeline()));
}
try {
publicClientApplicationBuilder = publicClientApplicationBuilder.authority(authorityUrl);
} catch (MalformedURLException e) {
Expand All @@ -110,11 +109,14 @@ public Mono<AccessToken> authenticateWithClientSecret(String clientSecret, Token
String authorityUrl = options.getAuthorityHost().replaceAll("/+$", "") + "/" + tenantId;
try {
ConfidentialClientApplication.Builder applicationBuilder =
ConfidentialClientApplication.builder(clientId, ClientCredentialFactory.create(clientSecret))
ConfidentialClientApplication.builder(clientId, ClientCredentialFactory.createFromSecret(clientSecret))
.authority(authorityUrl);
if (options.getProxyOptions() != null) {
applicationBuilder.proxy(proxyOptionsToJavaNetProxy(options.getProxyOptions()));
}
if (options.getHttpPipeline() != null) {
applicationBuilder.httpClient(new HttpPipelineAdapter(options.getHttpPipeline()));
}
ConfidentialClientApplication application = applicationBuilder.build();
return Mono.fromFuture(application.acquireToken(
ClientCredentialParameters.builder(new HashSet<>(request.getScopes()))
Expand All @@ -137,28 +139,22 @@ public Mono<AccessToken> authenticateWithClientSecret(String clientSecret, Token
public Mono<AccessToken> authenticateWithPfxCertificate(String pfxCertificatePath, String pfxCertificatePassword,
TokenRequestContext request) {
String authorityUrl = options.getAuthorityHost().replaceAll("/+$", "") + "/" + tenantId;
try {
return Mono.fromCallable(() -> {
ConfidentialClientApplication.Builder applicationBuilder =
ConfidentialClientApplication.builder(clientId,
ClientCredentialFactory.create(new FileInputStream(pfxCertificatePath), pfxCertificatePassword))
.authority(authorityUrl);
ConfidentialClientApplication.builder(clientId, ClientCredentialFactory.createFromCertificate(
new FileInputStream(pfxCertificatePath), pfxCertificatePassword))
.authority(authorityUrl);
if (options.getProxyOptions() != null) {
applicationBuilder.proxy(proxyOptionsToJavaNetProxy(options.getProxyOptions()));
}
ConfidentialClientApplication application = applicationBuilder.build();
return Mono.fromFuture(application.acquireToken(
ClientCredentialParameters.builder(new HashSet<>(request.getScopes()))
.build()))
.map(ar -> new AccessToken(ar.accessToken(), OffsetDateTime.ofInstant(ar.expiresOnDate().toInstant(),
ZoneOffset.UTC)));
} catch (CertificateException
| UnrecoverableKeyException
| NoSuchAlgorithmException
| KeyStoreException
| NoSuchProviderException
| IOException e) {
return Mono.error(e);
}
if (options.getHttpPipeline() != null) {
applicationBuilder.httpClient(new HttpPipelineAdapter(options.getHttpPipeline()));
}
return applicationBuilder.build();
}).flatMap(application -> Mono.fromFuture(application.acquireToken(
ClientCredentialParameters.builder(new HashSet<>(request.getScopes())).build())))
.map(ar -> new AccessToken(ar.accessToken(), OffsetDateTime.ofInstant(ar.expiresOnDate().toInstant(),
ZoneOffset.UTC)));
}

/**
Expand All @@ -173,12 +169,16 @@ public Mono<AccessToken> authenticateWithPemCertificate(String pemCertificatePat
try {
byte[] pemCertificateBytes = Files.readAllBytes(Paths.get(pemCertificatePath));
ConfidentialClientApplication.Builder applicationBuilder =
ConfidentialClientApplication.builder(clientId,
ClientCredentialFactory.create(CertificateUtil.privateKeyFromPem(pemCertificateBytes),
CertificateUtil.publicKeyFromPem(pemCertificateBytes))).authority(authorityUrl);
ConfidentialClientApplication.builder(clientId, ClientCredentialFactory.createFromCertificate(
CertificateUtil.privateKeyFromPem(pemCertificateBytes),
CertificateUtil.publicKeyFromPem(pemCertificateBytes)))
.authority(authorityUrl);
if (options.getProxyOptions() != null) {
applicationBuilder.proxy(proxyOptionsToJavaNetProxy(options.getProxyOptions()));
}
if (options.getHttpPipeline() != null) {
applicationBuilder.httpClient(new HttpPipelineAdapter(options.getHttpPipeline()));
}
ConfidentialClientApplication application = applicationBuilder.build();
return Mono.fromFuture(application.acquireToken(
ClientCredentialParameters.builder(new HashSet<>(request.getScopes()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.azure.identity.implementation;

import com.azure.core.http.HttpPipeline;
import com.azure.core.http.ProxyOptions;

import java.time.Duration;
Expand All @@ -19,6 +20,7 @@ public final class IdentityClientOptions {
private int maxRetry;
private Function<Duration, Duration> retryTimeout;
private ProxyOptions proxyOptions;
private HttpPipeline httpPipeline;

/**
* Creates an instance of IdentityClientOptions with default settings.
Expand Down Expand Up @@ -88,12 +90,29 @@ public ProxyOptions getProxyOptions() {
}

/**
* Specifies he options for proxy configuration.
* Specifies the options for proxy configuration.
* @param proxyOptions the options for proxy configuration
* @return IdentityClientOptions
*/
public IdentityClientOptions setProxyOptions(ProxyOptions proxyOptions) {
this.proxyOptions = proxyOptions;
return this;
}

/**
* @return the HttpPipeline to send all requests
*/
public HttpPipeline getHttpPipeline() {
return httpPipeline;
}

/**
* Specifies the HttpPipeline to send all requests. This setting overrides the others.
* @param httpPipeline the HttpPipeline to send all requests
* @return IdentityClientOptions
*/
public IdentityClientOptions setHttpPipeline(HttpPipeline httpPipeline) {
this.httpPipeline = httpPipeline;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ public class IdentityClientIntegrationTests {
private static final String AZURE_CLIENT_CERTIFICATE = "AZURE_CLIENT_CERTIFICATE";
private final TokenRequestContext request = new TokenRequestContext().addScopes("https://management.azure.com/.default");

@Ignore("Integration test")
@Ignore("Integration tests")
public void clientSecretCanGetToken() {
IdentityClient client = new IdentityClient(System.getenv(AZURE_TENANT_ID), System.getenv(AZURE_CLIENT_ID), new IdentityClientOptions().setProxyOptions(new ProxyOptions(Type.HTTP, new InetSocketAddress("localhost", 8888))));
IdentityClient client = new IdentityClient(System.getenv(AZURE_TENANT_ID), System.getenv(AZURE_CLIENT_ID), new IdentityClientOptions());
StepVerifier.create(client.authenticateWithClientSecret(System.getenv(AZURE_CLIENT_SECRET), request))
.expectNextMatches(token -> token.getToken() != null
&& token.getExpiresAt() != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
import com.azure.core.credential.AccessToken;
import com.azure.core.credential.TokenRequestContext;
import com.azure.identity.util.TestUtils;
import com.microsoft.aad.msal4j.AsymmetricKeyCredential;
import com.microsoft.aad.msal4j.ClientCredentialParameters;
import com.microsoft.aad.msal4j.ClientSecret;
import com.microsoft.aad.msal4j.ConfidentialClientApplication;
import com.microsoft.aad.msal4j.DeviceCodeFlowParameters;
import com.microsoft.aad.msal4j.IClientCredential;
import com.microsoft.aad.msal4j.IClientSecret;
import com.microsoft.aad.msal4j.MsalServiceException;
import com.microsoft.aad.msal4j.PublicClientApplication;
import org.junit.Assert;
Expand Down Expand Up @@ -154,9 +154,10 @@ private void mockForClientSecret(String secret, TokenRequestContext request, Str
ConfidentialClientApplication.Builder builder = PowerMockito.mock(ConfidentialClientApplication.Builder.class);
when(builder.build()).thenReturn(application);
when(builder.authority(any())).thenReturn(builder);
when(builder.httpClient(any())).thenReturn(builder);
whenNew(ConfidentialClientApplication.Builder.class).withAnyArguments().thenAnswer(invocation -> {
String cid = (String) invocation.getArguments()[0];
ClientSecret clientSecret = (ClientSecret) invocation.getArguments()[1];
IClientSecret clientSecret = (IClientSecret) invocation.getArguments()[1];
if (!clientId.equals(cid)) {
throw new MsalServiceException("Invalid clientId", "InvalidClientId");
}
Expand All @@ -182,13 +183,14 @@ private void mockForClientCertificate(TokenRequestContext request, String access
ConfidentialClientApplication.Builder builder = PowerMockito.mock(ConfidentialClientApplication.Builder.class);
when(builder.build()).thenReturn(application);
when(builder.authority(any())).thenReturn(builder);
when(builder.httpClient(any())).thenReturn(builder);
whenNew(ConfidentialClientApplication.Builder.class).withAnyArguments().thenAnswer(invocation -> {
String cid = (String) invocation.getArguments()[0];
AsymmetricKeyCredential keyCredential = (AsymmetricKeyCredential) invocation.getArguments()[1];
IClientCredential keyCredential = (IClientCredential) invocation.getArguments()[1];
if (!clientId.equals(cid)) {
throw new MsalServiceException("Invalid clientId", "InvalidClientId");
}
if (keyCredential == null || keyCredential.key() == null) {
if (keyCredential == null) {
throw new MsalServiceException("Invalid clientCertificate", "InvalidClientCertificate");
}
return builder;
Expand Down Expand Up @@ -216,6 +218,7 @@ private void mockForDeviceCodeFlow(TokenRequestContext request, String accessTok
PublicClientApplication.Builder builder = PowerMockito.mock(PublicClientApplication.Builder.class);
when(builder.build()).thenReturn(application);
when(builder.authority(any())).thenReturn(builder);
when(builder.httpClient(any())).thenReturn(builder);
whenNew(PublicClientApplication.Builder.class).withArguments(clientId).thenReturn(builder);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void setup() throws Exception {
accessAspect = new PersistentTokenCacheAccessAspect();

confApp = ConfidentialClientApplication.builder(TestConfiguration.CONFIDENTIAL_CLIENT_ID,
ClientCredentialFactory.create(TestConfiguration.CONFIDENTIAL_CLIENT_SECRET))
ClientCredentialFactory.createFromSecret(TestConfiguration.CONFIDENTIAL_CLIENT_SECRET))
.authority(TestConfiguration.TENANT_SPECIFIC_AUTHORITY)
.setTokenCacheAccessAspect(accessAspect)
.build();
Expand Down
Loading