Skip to content

Commit

Permalink
[github #4555][devops #1765500] fix according to comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
wangmingliang-ms committed Aug 31, 2020
1 parent cc690d4 commit 1948107
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public String getCurrentUserId() throws IOException {
}

@Override
protected String getDefaultTenantId() {
protected String getCurrentTenantId() {
return delegateADAuthManager.getCommonTenantId();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@

package com.microsoft.azuretools.sdkmanage;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.microsoft.azure.auth.AzureAuthHelper;
import com.microsoft.azure.auth.AzureTokenWrapper;
import com.microsoft.azure.common.exceptions.AzureExecutionException;
import com.microsoft.azure.common.utils.JsonUtils;
import com.microsoft.azure.credentials.AzureCliCredentials;
import com.microsoft.azure.management.Azure;
import com.microsoft.azuretools.adauth.PromptBehavior;
Expand All @@ -35,7 +35,6 @@
import com.microsoft.azuretools.authmanage.CommonSettings;
import com.microsoft.azuretools.authmanage.Environment;
import com.microsoft.azuretools.authmanage.models.AuthMethodDetails;
import com.microsoft.azuretools.azurecommons.helpers.NotNull;
import com.microsoft.azuretools.azurecommons.helpers.Nullable;
import com.microsoft.azuretools.utils.CommandUtils;
import com.microsoft.azuretools.utils.Pair;
Expand All @@ -55,18 +54,17 @@
import static com.microsoft.azuretools.authmanage.Environment.ENVIRONMENT_LIST;

public class AzureCliAzureManager extends AzureManagerBase {
private static final ObjectMapper MAPPER = new ObjectMapper();
private static final String FAILED_TO_AUTH_WITH_AZURE_CLI = "Failed to auth with Azure CLI";
private static final String UNABLE_TO_GET_AZURE_CLI_CREDENTIALS = "Unable to get Azure CLI credentials, " +
"please ensure you have installed Azure CLI and signed in.";
public static final String CLI_TOKEN_FORMAT_ACCESSOR = "az account get-access-token --output json -t %s";
public static final String CLI_TOKEN_PROP_ACCESS_TOKEN = "accessToken";
public static final String CLI_TOKEN_FORMAT_ACCESSOR = "az.cmd account get-access-token -t %s";
public static final String CLI_TOKEN_PROP_EXPIRATION = "expiresOn";

protected Map<String, Pair<String, OffsetDateTime>> tenantTokens = new ConcurrentHashMap<>();

private String defaultTenantId;
private String defaultClientId;
private String currentTenantId;
private String currentClientId;

static {
settings.setSubscriptionsDetailsFileName(FILE_NAME_SUBSCRIPTIONS_DETAILS_AZ);
Expand All @@ -77,57 +75,34 @@ public String getAccessToken(String tid, String resource, PromptBehavior promptB
if (!this.isSignedIn()) {
return null;
}
Pair<String, OffsetDateTime> token = tenantTokens.computeIfAbsent(tid, this::getAccessTokenViaCli);
Pair<String, OffsetDateTime> token = tenantTokens.get(tid);
final OffsetDateTime now = LocalDateTime.now().atZone(ZoneId.systemDefault()).toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
if (token.second().isBefore(now)) {
if (Objects.isNull(token) || token.second().isBefore(now)) {
token = this.getAccessTokenViaCli(tid);
tenantTokens.put(tid, token);
}
return token.first();
}

/**
* refer https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/
* identity/implementation/IdentityClient.java#L366
*/
private Pair<String, OffsetDateTime> getAccessTokenViaCli(String tid) {
//
final String command = String.format("az account get-access-token --output json -t %s", tid);
final String jsonToken;
try {
jsonToken = CommandUtils.exec(command);
} catch (IOException e) {
throw new RuntimeException(e);
}
final Map<String, Object> objectMap = convertJsonToMap(jsonToken);
final String strToken = (String) objectMap.get(CLI_TOKEN_PROP_ACCESS_TOKEN);
final String strTime = (String) objectMap.get(CLI_TOKEN_PROP_EXPIRATION);
final String decoratedTime = String.join("T", strTime.substring(0, strTime.indexOf(".")).split(" "));
final OffsetDateTime expiresOn = LocalDateTime.parse(decoratedTime, DateTimeFormatter.ISO_LOCAL_DATE_TIME)
.atZone(ZoneId.systemDefault())
.toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
return new Pair<>(strToken, expiresOn);
}

@Override
public String getCurrentUserId() {
return this.defaultClientId;
return this.currentClientId;
}

@Override
protected String getDefaultTenantId() {
return this.defaultTenantId;
protected String getCurrentTenantId() {
return this.currentTenantId;
}

@Override
public void drop() throws IOException {
this.defaultClientId = null;
this.defaultTenantId = null;
this.currentClientId = null;
this.currentTenantId = null;
super.drop();
}

public boolean isSignedIn() {
return Objects.nonNull(this.defaultTenantId) && Objects.nonNull(this.defaultClientId);
return Objects.nonNull(this.currentTenantId) && Objects.nonNull(this.currentClientId);
}

public AuthMethodDetails signIn() throws AzureExecutionException {
Expand All @@ -141,8 +116,8 @@ public AuthMethodDetails signIn() throws AzureExecutionException {
if (authenticated == null) {
throw new AzureExecutionException(FAILED_TO_AUTH_WITH_AZURE_CLI);
}
this.defaultClientId = credentials.clientId();
this.defaultTenantId = authenticated.tenantId();
this.currentClientId = credentials.clientId();
this.currentTenantId = authenticated.tenantId();
final Environment environment = ENVIRONMENT_LIST.stream()
.filter(e -> ObjectUtils.equals(credentials.environment(), e.getAzureEnvironment()))
.findAny()
Expand All @@ -163,15 +138,6 @@ public AuthMethodDetails signIn() throws AzureExecutionException {
}
}

private static <V> Map<String, V> convertJsonToMap(@NotNull String jsonString) {
try {
return MAPPER.readValue(jsonString, new TypeReference<Map<String, V>>() {
});
} catch (Exception ignore) {
return null;
}
}

public static AzureCliAzureManager getInstance() {
return LazyLoader.INSTANCE;
}
Expand All @@ -197,4 +163,21 @@ public AuthMethodDetails restore(final AuthMethodDetails authMethodDetails) {
private static class LazyLoader {
static final AzureCliAzureManager INSTANCE = new AzureCliAzureManager();
}

/**
* refer https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/
* identity/implementation/IdentityClient.java#L366
*/
private Pair<String, OffsetDateTime> getAccessTokenViaCli(String tid) throws IOException {
final String command = String.format(CLI_TOKEN_FORMAT_ACCESSOR, tid);
final String jsonToken = CommandUtils.exec(command);
final Map<String, Object> objectMap = JsonUtils.fromJson(jsonToken, Map.class);
final String strToken = (String) objectMap.get(CLI_TOKEN_PROP_ACCESS_TOKEN);
final String strTime = (String) objectMap.get(CLI_TOKEN_PROP_EXPIRATION);
final String decoratedTime = String.join("T", strTime.substring(0, strTime.indexOf(".")).split(" "));
final OffsetDateTime expiresOn = LocalDateTime.parse(decoratedTime, DateTimeFormatter.ISO_LOCAL_DATE_TIME)
.atZone(ZoneId.systemDefault())
.toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
return new Pair<>(strToken, expiresOn);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -124,7 +125,7 @@ public List<Subscription> getSubscriptions() throws IOException {
@Override
public List<Pair<Subscription, Tenant>> getSubscriptionsWithTenant() throws IOException {
final List<Pair<Subscription, Tenant>> subscriptions = new LinkedList<>();
final Azure.Authenticated authentication = authTenant(getDefaultTenantId());
final Azure.Authenticated authentication = authTenant(getCurrentTenantId());
// could be multi tenant - return all subscriptions for the current account
final List<Tenant> tenants = getTenants(authentication);
for (Tenant tenant : tenants) {
Expand Down Expand Up @@ -229,11 +230,11 @@ public Settings getSettings() {

@Override
public void drop() throws IOException {
System.out.println("ServicePrincipalAzureManager.drop()");
LOGGER.log(Level.INFO, "ServicePrincipalAzureManager.drop()");
this.subscriptionManager.cleanSubscriptions();
}

protected abstract String getDefaultTenantId() throws IOException;
protected abstract String getCurrentTenantId() throws IOException;

protected boolean isSignedIn() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public String getCurrentUserId() throws IOException {
}

@Override
protected String getDefaultTenantId() throws IOException {
protected String getCurrentTenantId() throws IOException {
return this.defaultTenantId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;
import sun.util.logging.PlatformLogger;

import java.io.*;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;

public class CommandUtils {
Expand Down Expand Up @@ -101,7 +103,7 @@ private static String executeCommandAndGetOutput(final CommandLine commandLine,
executor.setExitValues(null);
try {
executor.execute(commandLine);
System.err.println(err.toString());
logger.log(Level.SEVERE, err.toString());
return out.toString();
} catch (ExecuteException e) {
// swallow execute exception and return empty
Expand Down

0 comments on commit 1948107

Please sign in to comment.