Skip to content

Commit

Permalink
Enhance auth manager error handling (#4533)
Browse files Browse the repository at this point in the history 
* Enhance auth manager error handling

* Fix checkstyle

* Send telemetries for authentication

* Fix telemetry issue
  • Loading branch information
@Flanker32
Flanker32 authored Aug 7, 2020
1 parent 37be464 commit 907fa81
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 83 deletions.
146 changes: 77 additions & 69 deletions Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/AuthMethodManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,37 +30,40 @@
import com.microsoft.azuretools.azurecommons.helpers.Nullable;
import com.microsoft.azuretools.sdkmanage.AzureManager;
import com.microsoft.azuretools.sdkmanage.ServicePrincipalAzureManager;
import com.microsoft.azuretools.telemetrywrapper.ErrorType;
import com.microsoft.azuretools.telemetrywrapper.EventType;
import com.microsoft.azuretools.telemetrywrapper.EventUtil;
import com.microsoft.azuretools.utils.AzureUIRefreshCore;
import com.microsoft.azuretools.utils.AzureUIRefreshEvent;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.logging.Logger;

import static com.microsoft.azuretools.Constants.FILE_NAME_AUTH_METHOD_DETAILS;
import static com.microsoft.azuretools.telemetry.TelemetryConstants.ACCOUNT;
import static com.microsoft.azuretools.telemetry.TelemetryConstants.RESIGNIN;
import static com.microsoft.azuretools.telemetry.TelemetryConstants.SIGNIN_METHOD;

public class AuthMethodManager {
private static final Logger LOGGER = Logger.getLogger(AuthMethodManager.class.getName());
private static final String CANNOT_GET_AZURE_MANAGER = "Cannot get Azure Manager. "
+ "Please check if you have already signed in.";
private static final String CANNOT_GET_AZURE_BY_SID = "Cannot get Azure with Subscription ID: %s. "
+ "Please check if you have already signed in with this Subscription.";
private static final String CANNOT_GET_AZURE_MANAGER = "Cannot get Azure Manager. " +
"Please check if you have already signed in.";
private static final String CANNOT_GET_AZURE_BY_SID = "Cannot get Azure with Subscription ID: %s. " +
"Please check if you have already signed in with this Subscription.";
private static final String FAILED_TO_GET_AZURE_MANAGER_INSTANCE = "Failed to get an AzureManager instance " +
"for AuthMethodDetails: %s with error %s";

private AuthMethodDetails authMethodDetails;
private volatile AzureManager azureManager;
private Set<Runnable> signInEventListeners = new HashSet<>();
private Set<Runnable> signOutEventListeners = new HashSet<>();
private final Set<Runnable> signInEventListeners = new HashSet<>();
private final Set<Runnable> signOutEventListeners = new HashSet<>();

private AuthMethodManager() {
final AuthMethodDetails savedAuthMethodDetails = loadSettings();
authMethodDetails = savedAuthMethodDetails.getAuthMethod() == null ? new AuthMethodDetails() :
savedAuthMethodDetails.getAuthMethod().restoreAuth(savedAuthMethodDetails);
}

private static class LazyHolder {
static final AuthMethodManager INSTANCE = new AuthMethodManager();
private AuthMethodManager(AuthMethodDetails authMethodDetails) {
this.authMethodDetails = authMethodDetails;
}

public static AuthMethodManager getInstance() {
Expand All @@ -86,27 +89,19 @@ public AppPlatformManager getAzureSpringCloudClient(String sid) throws IOExcepti
}

public void addSignInEventListener(Runnable l) {
if (!signInEventListeners.contains(l)) {
signInEventListeners.add(l);
}
signInEventListeners.add(l);
}

public void removeSignInEventListener(Runnable l) {
if (signInEventListeners.contains(l)) {
signInEventListeners.remove(l);
}
signInEventListeners.remove(l);
}

public void addSignOutEventListener(Runnable l) {
if (!signOutEventListeners.contains(l)) {
signOutEventListeners.add(l);
}
signOutEventListeners.add(l);
}

public void removeSignOutEventListener(Runnable l) {
if (signOutEventListeners.contains(l)) {
signOutEventListeners.remove(l);
}
signOutEventListeners.remove(l);
}

public void notifySignInEventListener() {
Expand All @@ -132,29 +127,46 @@ public AzureManager getAzureManager() throws IOException {
return getAzureManager(getAuthMethod());
}

private synchronized AzureManager getAzureManager(final AuthMethod authMethod) throws IOException {
AzureManager localAzureManagerRef = azureManager;

if (localAzureManagerRef == null) {
try {
localAzureManagerRef = authMethod.createAzureManager(getAuthMethodDetails());
} catch (RuntimeException ex) {
LOGGER.info(String.format(
"Failed to get an AzureManager instance for AuthMethodDetails: %s with error %s",
getAuthMethodDetails(), ex.getMessage()));

cleanAll();
}
public void signOut() throws IOException {
cleanAll();
notifySignOutEventListener();
}

azureManager = localAzureManagerRef;
public boolean isSignedIn() {
try {
return getAzureManager() != null;
} catch (IOException e) {
return false;
}
}

return localAzureManagerRef;
public AuthMethod getAuthMethod() {
return authMethodDetails == null ? null : authMethodDetails.getAuthMethod();
}

public void signOut() throws IOException {
public AuthMethodDetails getAuthMethodDetails() {
return this.authMethodDetails;
}

public synchronized void setAuthMethodDetails(AuthMethodDetails authMethodDetails) throws IOException {
cleanAll();
notifySignOutEventListener();
this.authMethodDetails = authMethodDetails;
saveSettings();
}

private synchronized AzureManager getAzureManager(final AuthMethod authMethod) throws IOException {
if (authMethod == null) {
return null;
}
if (azureManager == null) {
try {
azureManager = authMethod.createAzureManager(getAuthMethodDetails());
} catch (RuntimeException ex) {
LOGGER.info(String.format(FAILED_TO_GET_AZURE_MANAGER_INSTANCE, getAuthMethodDetails(), ex.getMessage()));
cleanAll();
}
}
return azureManager;
}

private synchronized void cleanAll() throws IOException {
Expand All @@ -171,30 +183,34 @@ private synchronized void cleanAll() throws IOException {
saveSettings();
}

public boolean isSignedIn() {
try {
return getAzureManager() != null;
} catch (IOException e) {
return false;
}
}

public AuthMethod getAuthMethod() {
return authMethodDetails.getAuthMethod();
private void saveSettings() throws IOException {
System.out.println("saving authMethodDetails...");
String sd = JsonHelper.serialize(authMethodDetails);
FileStorage fs = new FileStorage(FILE_NAME_AUTH_METHOD_DETAILS, CommonSettings.getSettingsBaseDir());
fs.write(sd.getBytes(StandardCharsets.UTF_8));
}

public AuthMethodDetails getAuthMethodDetails() {
return this.authMethodDetails;
private static class LazyHolder {
static final AuthMethodManager INSTANCE = initAuthMethodManagerFromSettings();
}

public synchronized void setAuthMethodDetails(AuthMethodDetails authMethodDetails) throws IOException {
cleanAll();
this.authMethodDetails = authMethodDetails;
saveSettings();
//if (isSignedIn()) notifySignInEventListener();
private static AuthMethodManager initAuthMethodManagerFromSettings() {
return EventUtil.executeWithLog(ACCOUNT, RESIGNIN, operation -> {
try {
final AuthMethodDetails savedAuthMethodDetails = loadSettings();
final AuthMethodDetails authMethodDetails = savedAuthMethodDetails.getAuthMethod() == null ?
new AuthMethodDetails() : savedAuthMethodDetails.getAuthMethod().restoreAuth(savedAuthMethodDetails);
final String authMethod = authMethodDetails.getAuthMethod() == null ? "Empty" : authMethodDetails.getAuthMethod().name();
EventUtil.logEvent(EventType.info, operation, Collections.singletonMap(SIGNIN_METHOD, authMethod));
return new AuthMethodManager(authMethodDetails);
} catch (RuntimeException ignore) {
EventUtil.logError(operation, ErrorType.systemError, ignore, null, null);
return new AuthMethodManager(new AuthMethodDetails());
}
});
}

private AuthMethodDetails loadSettings() {
private static AuthMethodDetails loadSettings() {
System.out.println("loading authMethodDetails...");
try {
FileStorage fs = new FileStorage(FILE_NAME_AUTH_METHOD_DETAILS, CommonSettings.getSettingsBaseDir());
Expand All @@ -204,18 +220,10 @@ private AuthMethodDetails loadSettings() {
System.out.println(FILE_NAME_AUTH_METHOD_DETAILS + " is empty");
return new AuthMethodDetails();
}

return JsonHelper.deserialize(AuthMethodDetails.class, json);
} catch (IOException ignored) {
System.out.println("Failed to loading authMethodDetails settings. Use defaults.");
return new AuthMethodDetails();
}
}

private void saveSettings() throws IOException {
System.out.println("saving authMethodDetails...");
String sd = JsonHelper.serialize(authMethodDetails);
FileStorage fs = new FileStorage(FILE_NAME_AUTH_METHOD_DETAILS, CommonSettings.getSettingsBaseDir());
fs.write(sd.getBytes(StandardCharsets.UTF_8));
}
}
5 changes: 1 addition & 4 deletions Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/models/AuthMethodDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@

package com.microsoft.azuretools.authmanage.models;

import com.microsoft.azuretools.authmanage.AuthMethod;
import org.codehaus.jackson.annotate.JsonIgnoreProperties;
import org.codehaus.jackson.annotate.JsonProperty;

import com.microsoft.azuretools.authmanage.AuthMethod;

/**
* Created by shch on 10/8/2016.
*/
Expand All @@ -47,7 +46,6 @@ public class AuthMethodDetails {

// for jackson json
public AuthMethodDetails() {
this.authMethod = AuthMethod.AD;
}

public AuthMethod getAuthMethod() {
Expand All @@ -74,7 +72,6 @@ public void setCredFilePath(String credFilePath) {
this.credFilePath = credFilePath;
}


public String getAzureEnv() {
return azureEnv;
}
Expand Down
13 changes: 8 additions & 5 deletions Utils/azuretools-core/src/com/microsoft/azuretools/telemetry/AppInsightsClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.microsoft.azuretools.adauth.StringUtils;
import com.microsoft.azuretools.azurecommons.helpers.Nullable;
import com.microsoft.azuretools.telemetrywrapper.TelemetryManager;

import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
Expand All @@ -47,7 +48,7 @@ public enum EventType {
Azure
}

public static String getInstallationId(){
public static String getInstallationId() {
return configuration == null ? null : configuration.installationId();
}

Expand Down Expand Up @@ -77,7 +78,8 @@ public static void createByType(final EventType eventType, final String objectNa
createByType(eventType, objectName, action, properties, false);
}

public static void createByType(final EventType eventType, final String objectName, final String action, final Map<String, String> properties, final boolean force) {
public static void createByType(final EventType eventType, final String objectName, final String action, final Map<String, String> properties,
final boolean force) {
if (!isAppInsightsClientAvailable())
return;

Expand Down Expand Up @@ -108,7 +110,7 @@ public static void create(String eventName, String version, @Nullable Map<String
}

private static void create(String eventName, String version, @Nullable Map<String, String> myProperties,
Map<String, Double> metrics, boolean force) {
Map<String, Double> metrics, boolean force) {
if (isAppInsightsClientAvailable() && configuration.validated()) {
String prefValue = configuration.preferenceVal();
if (prefValue == null || prefValue.isEmpty() || prefValue.equalsIgnoreCase("true") || force) {
Expand All @@ -128,7 +130,7 @@ private static Map<String, String> buildProperties(String version, Map<String, S
properties.put("IDE", configuration.ide());

// Telemetry client doesn't accept null value for ConcurrentHashMap doesn't accept null as key or value..
for (Iterator<Map.Entry<String, String>> iter = properties.entrySet().iterator(); iter.hasNext(); ) {
for (Iterator<Map.Entry<String, String>> iter = properties.entrySet().iterator(); iter.hasNext();) {
Map.Entry<String, String> entry = iter.next();
if (StringUtils.isNullOrEmpty(entry.getKey()) || StringUtils.isNullOrEmpty(entry.getValue())) {
iter.remove();
Expand Down Expand Up @@ -177,7 +179,7 @@ public static void createFTPEvent(String eventName, String uri, String appName,
properties.put("Installation ID", instID);
}
}
synchronized(TelemetryClientSingleton.class){
synchronized (TelemetryClientSingleton.class) {
telemetry.trackEvent(eventName, properties, null);
telemetry.flush();
}
Expand All @@ -192,6 +194,7 @@ private static void initTelemetryManager() {
TelemetryManager.getInstance().setCommonProperties(buildProperties("", new HashMap<>()));
TelemetryManager.getInstance().setTelemetryClient(TelemetryClientSingleton.getTelemetry());
TelemetryManager.getInstance().setEventNamePrefix(configuration.eventName());
TelemetryManager.getInstance().sendCachedTelemetries();
} catch (Exception ignore) {
}
}
Expand Down
1 change: 1 addition & 0 deletions Utils/azuretools-core/src/com/microsoft/azuretools/telemetry/TelemetryConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public class TelemetryConstants {
public static final String REFRESH_METADATA = "refresh";
public static final String SIGNIN = "signin";
public static final String SIGNOUT = "signout";
public static final String RESIGNIN = "re-signin";
public static final String SELECT_SUBSCRIPTIONS = "select-subscriptions";
public static final String GET_SUBSCRIPTIONS = "get-subscriptions";
public static final String REPORT_ISSUES = "report-issues";
Expand Down
29 changes: 24 additions & 5 deletions Utils/azuretools-core/src/com/microsoft/azuretools/telemetrywrapper/CommonUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@

import com.microsoft.applicationinsights.TelemetryClient;
import com.microsoft.azuretools.adauth.StringUtils;
import org.apache.commons.lang3.tuple.MutableTriple;
import org.joda.time.Instant;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class CommonUtil {
Expand All @@ -42,6 +45,7 @@ public class CommonUtil {
public static final String SERVICE_NAME = "serviceName";
public static final String TIMESTAMP = "timestamp";
public static TelemetryClient client;
private static List<MutableTriple<EventType, Map, Map>> cachedEvents = new ArrayList<>();

public static Map<String, String> mergeProperties(Map<String, String> properties) {
Map<String, String> commonProperties = TelemetryManager.getInstance().getCommonProperties();
Expand All @@ -52,20 +56,35 @@ public static Map<String, String> mergeProperties(Map<String, String> properties
return merged;
}

public synchronized static void sendTelemetry(EventType eventType, String serviceName, Map<String, String> properties,
public static synchronized void sendTelemetry(EventType eventType, String serviceName, Map<String, String> properties,
Map<String, Double> metrics) {
Map<String, String> mutableProps = properties == null ? new HashMap<>() : new HashMap<>(properties);
// Tag UTC time as timestamp
mutableProps.put(TIMESTAMP, Instant.now().toString());
if (!StringUtils.isNullOrEmpty(serviceName)) {
mutableProps.put(SERVICE_NAME, serviceName);
}
if (client != null) {
final String eventName = getFullEventName(eventType);
client.trackEvent(eventName, mutableProps, metrics);
client.flush();
} else {
cacheEvents(eventType, mutableProps, metrics);
}
}

public static void clearCachedEvents() {
if (client != null) {
if (!StringUtils.isNullOrEmpty(serviceName)) {
mutableProps.put(SERVICE_NAME, serviceName);
}
client.trackEvent(getFullEventName(eventType), mutableProps, metrics);
cachedEvents.forEach(triple -> client.trackEvent(getFullEventName(triple.left), triple.middle, triple.right));
client.flush();
cachedEvents.clear();
}
}

private static void cacheEvents(EventType eventType, Map<String, String> mutableProps, Map<String, Double> metrics) {
cachedEvents.add(MutableTriple.of(eventType, mutableProps, metrics));
}

private static String getFullEventName(EventType eventType) {
return TelemetryManager.getInstance().getEventNamePrefix() + "/" + eventType.name();
}
Expand Down
Loading

0 comments on commit 907fa81

Please sign in to comment.