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

Persistence token cache for Mac & Linux with new MSAL ext #9188

Merged
merged 55 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
898a02f
Depend on msal persistence extension
jianghaolu Mar 17, 2020
132c01a
Initial draft of shared token cache for Mac and Linux
jianghaolu Mar 18, 2020
3c03443
Clean up
jianghaolu Mar 18, 2020
49acab7
Add msal ext to module-info
jianghaolu Mar 18, 2020
f50d8b7
Wrap MSAL error and fix module-info
jianghaolu Mar 18, 2020
4e4c467
checkstyle
jianghaolu Mar 18, 2020
fca0133
Fix spotbugs and naming
jianghaolu Mar 18, 2020
571b44c
Fix default azure credential test
jianghaolu Mar 23, 2020
e2c5dc3
Add initial perf test
jianghaolu Apr 1, 2020
dd341e7
more perf testing work
jianghaolu Apr 3, 2020
7cba579
Move default to identity client options
jianghaolu Apr 6, 2020
600fc6b
Fix array access in identity client options
jianghaolu Apr 7, 2020
1c53b24
Fix libsecret on Windows
jianghaolu Apr 7, 2020
3539779
Remove shared token cache configurations
jianghaolu Apr 7, 2020
80bb671
Clean up
jianghaolu Apr 7, 2020
b289411
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 7, 2020
a83c9ab
Fix tests
jianghaolu Apr 7, 2020
52e11f6
Fix versions
jianghaolu Apr 7, 2020
5a79873
Fix versions
jianghaolu Apr 7, 2020
bfcf3b3
Add version again
jianghaolu Apr 8, 2020
3660848
Lazy initialize pub client
jianghaolu Apr 8, 2020
8c5606a
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 8, 2020
435a4f2
Defer Mono.fromFuture()
jianghaolu Apr 8, 2020
4ad8ef8
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 8, 2020
5ac9807
Checkstyle
jianghaolu Apr 8, 2020
0b48bbb
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 13, 2020
f8245f0
Fix in perf test
jianghaolu Apr 14, 2020
e8d572a
Merge branch 'persistence' of github.com:jianghaolu/azure-sdk-for-jav…
jianghaolu Apr 14, 2020
ed6b013
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 20, 2020
1472726
Does not throw except SharedTokenCacheCredential
jianghaolu Apr 21, 2020
1cc8254
Fix merge error
jianghaolu Apr 21, 2020
6b1e50d
x-include-update
jianghaolu Apr 21, 2020
3a16cad
Minor change in public client creation
jianghaolu Apr 22, 2020
db0eb37
Checkstyle
jianghaolu Apr 22, 2020
775568f
Revert persistent cache demo
jianghaolu Apr 22, 2020
006b191
Align Linux default settings with MSAL.NET tests
jianghaolu Apr 22, 2020
eb9c615
Fix readme and use expandable enum
jianghaolu Apr 23, 2020
abfbea9
Merge branch 'persistence' of github.com:jianghaolu/azure-sdk-for-jav…
jianghaolu Apr 25, 2020
233ce5c
Disable shared token cache by default
jianghaolu Apr 29, 2020
a0b1725
Allow enabling on select builders
jianghaolu Apr 29, 2020
77c8a59
Wrap exceptions in ClientAuthenticationExceptions when Msal fails
jianghaolu Apr 29, 2020
7c7045c
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu Apr 29, 2020
ed8e722
Address spotbugs
jianghaolu Apr 29, 2020
09718dd
Clean up and address Alan's review
jianghaolu Apr 29, 2020
806539a
Merge error message
jianghaolu Apr 29, 2020
066d413
CI says IdentityClient doesn't compile
jianghaolu Apr 29, 2020
6cc8c1b
Checkstyle: indentation
jianghaolu Apr 29, 2020
d5c4662
Update shared token cache look up with master
jianghaolu Apr 30, 2020
0bf3f76
Address Connie's feedback
jianghaolu May 1, 2020
b337b93
address feedback
g2vinay May 1, 2020
4e1d1a0
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu May 3, 2020
5b4e878
Merge branch 'master' of github.com:Azure/azure-sdk-for-java into per…
jianghaolu May 4, 2020
25d2a63
Fix shared token cache and error handling
jianghaolu May 4, 2020
e3a405d
Use getPublicClientApplication()
jianghaolu May 4, 2020
3dff1f3
Undo changes in key vault
jianghaolu May 4, 2020
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
1 change: 1 addition & 0 deletions eng/versioning/external_dependencies.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ 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;1.3.0
com.microsoft.azure:msal4j-persistence-extension;0.1
com.sun.activation:jakarta.activation;1.2.1
io.opentelemetry:opentelemetry-api;0.2.4
io.opentelemetry:opentelemetry-sdk;0.2.4
Expand Down
1 change: 1 addition & 0 deletions eng/versioning/version_client.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ com.azure:azure-cosmos-benchmark;4.0.1-beta.1;4.0.1-beta.1
com.azure:azure-data-appconfiguration;1.1.1;1.2.0-beta.1
com.azure:azure-e2e;1.0.0-beta.1;1.0.0-beta.1
com.azure:azure-identity;1.0.4;1.1.0-beta.3
com.azure:azure-identity-perf;1.0.0-beta.1;1.0.0-beta.1
com.azure:azure-messaging-eventhubs;5.0.2;5.1.0-beta.1
com.azure:azure-messaging-eventhubs-checkpointstore-blob;1.0.2;1.1.0-beta.1
com.azure:azure-messaging-servicebus;7.0.0-beta.1;7.0.0-beta.2
Expand Down
6 changes: 6 additions & 0 deletions sdk/identity/azure-identity/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
<artifactId>msal4j</artifactId>
<version>1.3.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
</dependency>
<dependency>
<groupId>com.microsoft.azure</groupId>
<artifactId>msal4j-persistence-extension</artifactId>
<version>0.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j-persistence-extension;external_dependency} -->
</dependency>
<dependency>
<groupId>com.nimbusds</groupId>
<artifactId>oauth2-oidc-sdk</artifactId>
Expand Down Expand Up @@ -122,6 +127,7 @@
<include>com.azure:*</include>
<include>com.nimbusds:oauth2-oidc-sdk</include>
<include>com.microsoft.azure:msal4j</include>
<include>com.microsoft.azure:msal4j-persistence-extension</include>
<include>org.nanohttpd:nanohttpd</include>
<include>net.java.dev.jna</include>
</includes>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,31 @@ public T executorService(ExecutorService executorService) {
this.identityClientOptions.setExecutorService(executorService);
return (T) this;
}

/**
* Sets whether to use an unprotected file specified by <code>cacheFileLocation()</code> instead of
* Gnome keyring on Linux. This is false by default.
*
* @param useUnprotectedFileOnLinux whether to use an unprotected file for cache storage.
*
* @return The updated T object.
jianghaolu marked this conversation as resolved.
Show resolved Hide resolved
*/
@SuppressWarnings("unchecked")
public T useUnprotectedTokenCacheFileOnLinux(boolean useUnprotectedFileOnLinux) {
this.identityClientOptions.setUseUnprotectedTokenCacheFileOnLinux(useUnprotectedFileOnLinux);
return (T) this;
}

/**
* Disable using the shared token cache.
*
* @param disabled whether to disable using the shared token cache.
*
* @return The updated identity client options.
jianghaolu marked this conversation as resolved.
Show resolved Hide resolved
*/
@SuppressWarnings("unchecked")
public T disableSharedTokenCache(boolean disabled) {
Copy link
Member

Choose a reason for hiding this comment

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

I think that persistence of the token cache should be opt-in rather than opt out. Also I'm not sure we can put this on the base builder for all credentials, for instance what does it mean to create a SharedTokenCachceCredential with the shared token cache disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For SharedTokenCacheCredential, yes you can disable shared token cache, but assuming no one will do this does save us a ton of duplicate code.

Copy link
Member

Choose a reason for hiding this comment

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

ManagedIdentityCredential and DefaultAzureCredential inherit from CredentialBuilderBase, so they will not inherit this setter currently.

this.identityClientOptions.disableSharedTokenCache(disabled);
return (T) this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,21 @@
*
* <p><strong>Sample: Construct a ChainedTokenCredential with silent username+password login tried first, then
* interactive browser login as needed (e.g. when 2FA is turned on in the directory).</strong></p>
* {@codesnippet com.azure.identity.credential.chainedtokencredential.construct}
* <pre>
* UsernamePasswordCredential usernamePasswordCredential = new UsernamePasswordCredentialBuilder&#40;&#41;
* .clientId&#40;clientId&#41;
* .username&#40;username&#41;
* .password&#40;password&#41;
* .build&#40;&#41;;
* InteractiveBrowserCredential interactiveBrowserCredential = new InteractiveBrowserCredentialBuilder&#40;&#41;
* .clientId&#40;clientId&#41;
* .port&#40;8765&#41;
* .build&#40;&#41;;
* ChainedTokenCredential credential = new ChainedTokenCredentialBuilder&#40;&#41;
* .addLast&#40;usernamePasswordCredential&#41;
* .addLast&#40;interactiveBrowserCredential&#41;
* .build&#40;&#41;;
* </pre>
*/
@Immutable
public class ChainedTokenCredential implements TokenCredential {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,23 @@
* An AAD credential that acquires a token with a client certificate for an AAD application.
*
* <p><strong>Sample: Construct a simple ClientCertificateCredential</strong></p>
* {@codesnippet com.azure.identity.credential.clientcertificatecredential.construct}
* <pre>
* ClientCertificateCredential credential1 = new ClientCertificateCredentialBuilder&#40;&#41;
* .tenantId&#40;tenantId&#41;
* .clientId&#40;clientId&#41;
* .pemCertificate&#40;&quot;C:&#92;&#92;fakepath&#92;&#92;cert.pem&quot;&#41;
* .build&#40;&#41;;
* </pre>
*
* <p><strong>Sample: Construct a ClientCertificateCredential behind a proxy</strong></p>
* {@codesnippet com.azure.identity.credential.clientcertificatecredential.constructwithproxy}
* <pre>
* ClientCertificateCredential credential2 = new ClientCertificateCredentialBuilder&#40;&#41;
* .tenantId&#40;tenantId&#41;
* .clientId&#40;clientId&#41;
* .pfxCertificate&#40;&quot;C:&#92;&#92;fakepath&#92;&#92;cert.pfx&quot;, &quot;P&#123;@literal @&#125;s$w0rd&quot;&#41;
* .proxyOptions&#40;new ProxyOptions&#40;Type.HTTP, new InetSocketAddress&#40;&quot;10.21.32.43&quot;, 5465&#41;&#41;&#41;
* .build&#40;&#41;;
* </pre>
*/
@Immutable
public class ClientCertificateCredential implements TokenCredential {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,23 @@
* An AAD credential that acquires a token with a client secret for an AAD application.
*
* <p><strong>Sample: Construct a simple ClientSecretCredential</strong></p>
* {@codesnippet com.azure.identity.credential.clientsecretcredential.construct}
* <pre>
* ClientSecretCredential credential1 = new ClientSecretCredentialBuilder&#40;&#41;
* .tenantId&#40;tenantId&#41;
* .clientId&#40;clientId&#41;
* .clientSecret&#40;clientSecret&#41;
* .build&#40;&#41;;
* </pre>
*
* <p><strong>Sample: Construct a ClientSecretCredential behind a proxy</strong></p>
* {@codesnippet com.azure.identity.credential.clientsecretcredential.constructwithproxy}
* <pre>
* ClientSecretCredential credential2 = new ClientSecretCredentialBuilder&#40;&#41;
* .tenantId&#40;tenantId&#41;
* .clientId&#40;clientId&#41;
* .clientSecret&#40;clientSecret&#41;
* .proxyOptions&#40;new ProxyOptions&#40;Type.HTTP, new InetSocketAddress&#40;&quot;10.21.32.43&quot;, 5465&#41;&#41;&#41;
* .build&#40;&#41;;
* </pre>
*/
@Immutable
public class ClientSecretCredential implements TokenCredential {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
*/
@Immutable
public final class DefaultAzureCredential extends ChainedTokenCredential {

/**
* Creates default DefaultAzureCredential instance to use. This will use AZURE_CLIENT_ID,
* AZURE_CLIENT_SECRET, and AZURE_TENANT_ID environment variables to create a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ private ArrayDeque<TokenCredential> getCredentialsChain() {
}

if (!excludeSharedTokenCacheCredential) {
output.add(new SharedTokenCacheCredential(null, null,
"04b07795-8ddb-461a-bbee-02f9e1bf7b46", identityClientOptions));
output.add(new SharedTokenCacheCredential(null, "04b07795-8ddb-461a-bbee-02f9e1bf7b46",
null, identityClientOptions));
}

if (!excludeAzureCliCredential) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.identity;

/**
* An expandable enum for types of item schema in a Keyring.
*/
public final class KeyringItemSchema {
jianghaolu marked this conversation as resolved.
Show resolved Hide resolved
public static final KeyringItemSchema GENERIC_SECRET = new KeyringItemSchema("org.freedesktop.Secret.Generic");
public static final KeyringItemSchema NETWORK_PASSWORD = new KeyringItemSchema(
"org.gnome.keyring.NetworkPassword");
public static final KeyringItemSchema NOTE = new KeyringItemSchema("org.gnome.keyring.Note");

private final String value;

private KeyringItemSchema(String value) {
this.value = value;
}

/**
* Parses a String into a new Keyring schema.
* @param schema the full name of the schema
* @return the KeyringItemSchema enum representing this schema
*/
public static KeyringItemSchema fromString(String schema) {
return new KeyringItemSchema(schema);
}

@Override
public String toString() {
return value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,11 @@
import com.azure.core.credential.TokenCredential;
import com.azure.core.credential.TokenRequestContext;
import com.azure.core.util.Configuration;
import com.azure.identity.implementation.IdentityClient;
import com.azure.identity.implementation.IdentityClientBuilder;
import com.azure.identity.implementation.IdentityClientOptions;
import com.azure.identity.implementation.msalextensions.PersistentTokenCacheAccessAspect;
import com.microsoft.aad.msal4j.IAccount;
import com.microsoft.aad.msal4j.IAuthenticationResult;
import com.microsoft.aad.msal4j.PublicClientApplication;
import com.microsoft.aad.msal4j.SilentParameters;
import reactor.core.publisher.Mono;

import java.net.MalformedURLException;
import java.time.ZoneOffset;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;

/**
* A credential provider that provides token credentials from the MSAL shared token cache.
* Requires a username and client Id. If a username is not provided, then the
Expand All @@ -34,7 +23,7 @@ public class SharedTokenCacheCredential implements TokenCredential {
private final String tenantId;
private final IdentityClientOptions options;

private PublicClientApplication pubClient = null;
private final IdentityClient identityClient;

/**
* Creates an instance of the Shared Token Cache Credential Provider.
Expand Down Expand Up @@ -64,86 +53,18 @@ public class SharedTokenCacheCredential implements TokenCredential {
this.tenantId = tenantId;
}
this.options = identityClientOptions;
this.identityClient = new IdentityClientBuilder()
.tenantId(this.tenantId)
.clientId(this.clientId)
.identityClientOptions(identityClientOptions)
.build();
}

/**
* Gets token from shared token cache
* */
@Override
public Mono<AccessToken> getToken(TokenRequestContext request) {
String authorityUrl = options.getAuthorityHost().replaceAll("/+$", "") + "/" + tenantId + "/";
// Initialize here so that the constructor doesn't throw
if (pubClient == null) {
try {
PersistentTokenCacheAccessAspect accessAspect = new PersistentTokenCacheAccessAspect();
PublicClientApplication.Builder applicationBuilder = PublicClientApplication.builder(this.clientId);
if (options.getExecutorService() != null) {
applicationBuilder.executorService(options.getExecutorService());
}

pubClient = applicationBuilder
.authority(authorityUrl)
.setTokenCacheAccessAspect(accessAspect)
.build();
} catch (Exception e) {
return Mono.error(e);
}
}

// find if the Public Client app with the requested username exists
return Mono.fromFuture(pubClient.getAccounts())
.flatMap(set -> {
IAccount requestedAccount;
Map<String, IAccount> accounts = new HashMap<>(); // home account id -> account

for (IAccount cached : set) {
if (username == null || username.equals(cached.username())) {
if (!accounts.containsKey(cached.homeAccountId())) { // only put the first one
accounts.put(cached.homeAccountId(), cached);
}
}
}

if (accounts.size() == 0) {
if (username == null) {
return Mono.error(new RuntimeException("No accounts were discovered in the shared token cache."
+ " To fix, authenticate through tooling supporting azure developer sign on."));
} else {
return Mono.error(new RuntimeException(String.format("User account '%s' was not found in the "
+ "shared token cache. Discovered Accounts: [ '%s' ]", username, set.stream()
.map(IAccount::username).distinct().collect(Collectors.joining(", ")))));
}
} else if (accounts.size() > 1) {
if (username == null) {
return Mono.error(new RuntimeException("Multiple accounts were discovered in the shared token "
+ "cache. To fix, set the AZURE_USERNAME and AZURE_TENANT_ID environment variable to the "
+ "preferred username, or specify it when constructing SharedTokenCacheCredential."));
} else {
return Mono.error(new RuntimeException("Multiple entries for the user account " + username
+ " were found in the shared token cache. This is not currently supported by the"
+ " SharedTokenCacheCredential."));
}
} else {
requestedAccount = accounts.values().iterator().next();
}

// if it does, then request the token
SilentParameters params = SilentParameters.builder(
new HashSet<>(request.getScopes()), requestedAccount)
.authorityUrl(authorityUrl)
.build();

CompletableFuture<IAuthenticationResult> future;
try {
future = pubClient.acquireTokenSilently(params);
return Mono.fromFuture(() -> future).map(result ->
new AccessToken(result.accessToken(),
result.expiresOnDate().toInstant().atOffset(ZoneOffset.UTC)));

} catch (MalformedURLException e) {
e.printStackTrace();
return Mono.error(new RuntimeException("Token was not found"));
}
});
return identityClient.authenticateWithSharedTokenCache(request, username);
jianghaolu marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
public class SharedTokenCacheCredentialBuilder extends AadCredentialBuilderBase<SharedTokenCacheCredentialBuilder> {
private String username;


/**
* Sets the username for the account.
*
Expand Down
Loading