From 809ee45ef4dad06666cbe485c057143d80b0d826 Mon Sep 17 00:00:00 2001 From: Summer Date: Tue, 27 Nov 2018 13:43:13 +0800 Subject: [PATCH] Remove unnecessary properties of AuthContext (#2371) --- .../azuretools/adauth/AuthContext.java | 49 +++++++++---------- .../azuretools/authmanage/AdAuthManager.java | 45 +++++++++-------- .../authmanage/BaseADAuthManager.java | 6 +-- .../azuretools/authmanage/DCAuthManager.java | 2 +- 4 files changed, 51 insertions(+), 51 deletions(-) diff --git a/Utils/azuretools-core/src/com/microsoft/azuretools/adauth/AuthContext.java b/Utils/azuretools-core/src/com/microsoft/azuretools/adauth/AuthContext.java index dffd14b56d7..97a358c8d50 100644 --- a/Utils/azuretools-core/src/com/microsoft/azuretools/adauth/AuthContext.java +++ b/Utils/azuretools-core/src/com/microsoft/azuretools/adauth/AuthContext.java @@ -45,13 +45,9 @@ public class AuthContext { private static final Logger log = Logger.getLogger(AuthContext.class.getName()); - // TODO: move the cast later after package change - private final IWebUi webUi; - private final String clientId; private final String authority; private UUID correlationId; - private final String redirectUrl; private final AuthenticationAuthority authenticationAuthority; private final boolean validateAuthority; private CacheDriver driver; @@ -64,24 +60,17 @@ public static void cleanTokenCache() { * AuthContext to acquire token. * @param authority String not null. * @param clientId String not null. - * @param redirectUrl String not null. - * @param webUi IWebUi ui to get auth token interactively. * @param validateAuthority boolean whether to validate the authority url. * @param correlationId UUID request correlation id. * @throws MalformedURLException exception thrown when parse url from authority. */ - // todo: webUi should not be a property of AuthContext, it is only used for interactive login - // todo: make it nullable as first step, then remove it and get it as parameter when acquire token by auth code - public AuthContext(@NotNull String authority, @NotNull String clientId, @NotNull String redirectUrl, - IWebUi webUi, final boolean validateAuthority, UUID correlationId) - throws MalformedURLException { + public AuthContext(@NotNull final String authority, @NotNull final String clientId, + final boolean validateAuthority, final UUID correlationId) throws MalformedURLException { this.authority = this.canonicalizeUri(authority); this.clientId = clientId; this.correlationId = (correlationId != null) ? correlationId : UUID.randomUUID(); - this.redirectUrl = redirectUrl; this.validateAuthority = validateAuthority; authenticationAuthority = new AuthenticationAuthority(new URL(this.authority), this.validateAuthority); - this.webUi = webUi; initDriver(); } @@ -100,10 +89,15 @@ public String getClientId() { * @return authentication result with updated tokens * @throws AuthException exception during getting token */ - public AuthResult acquireToken(@NotNull AuthResult lastResult) throws AuthException { + public AuthResult acquireToken(@NotNull final AuthResult lastResult) throws AuthException { driver.createAddEntry(lastResult, null); - return acquireToken(lastResult.getResource(), false, lastResult.getUserId(), lastResult.isMultipleResourceRefreshToken()); + final AuthResult result = acquireTokenFromCache(lastResult.getResource(), lastResult.getUserId()); + if (result != null) { + return result; + } else { + throw new AuthException(AuthError.UnknownUser); + } } /** @@ -112,18 +106,21 @@ public AuthResult acquireToken(@NotNull AuthResult lastResult) throws AuthExcept * @param newAuthCode String need to get new auth code. * @param userId String userId. * @param isDisplayable boolean whether the userId is displayable id. + * @param webUi web ui + * @param redirectUrl redirect url * @return AuthResult auth result with the tokens. * @throws AuthException exception during get token. */ - public AuthResult acquireToken(@NotNull String resource, boolean newAuthCode, - String userId, boolean isDisplayable) throws AuthException { + public AuthResult acquireToken(@NotNull final String resource, final boolean newAuthCode, final String userId, + final boolean isDisplayable, final IWebUi webUi, + final String redirectUrl) throws AuthException { String userDisplayableId = null; if (null != userId && isDisplayable) { userDisplayableId = userId; } if (newAuthCode) { - String code = acquireAuthCode(resource, userDisplayableId); - return getTokenWithAuthCode(code, resource); + String code = acquireAuthCode(resource, userDisplayableId, webUi, redirectUrl); + return getTokenWithAuthCode(code, resource, redirectUrl); } else { AuthResult result = null; result = acquireTokenFromCache(resource, userId); @@ -182,11 +179,12 @@ public AuthResult acquireToken(@NotNull final String resource, final boolean new return authResult; } - private String acquireAuthCode(@NotNull final String resource, String userDisplayableId) throws AuthException { + private String acquireAuthCode(@NotNull final String resource, final String userDisplayableId, + final IWebUi webUi, final String redirectUrl) throws AuthException { AuthCode code = null; try { AuthCodeInteractiveHandler handler = new AuthCodeInteractiveHandler(this.authenticationAuthority, - this.clientId, this.webUi, this.redirectUrl, resource, userDisplayableId); + this.clientId, webUi, redirectUrl, resource, userDisplayableId); String response = handler.acquireAuthCode(this.correlationId); code = parseAuthorizeResponse(response, this.correlationId); } catch (Exception e) { @@ -195,7 +193,7 @@ private String acquireAuthCode(@NotNull final String resource, String userDispla } log.log(Level.FINEST, "==> authorization code: " + code.getCode()); - + if (code.getStatus() == AuthorizationStatus.Success) { return code.getCode(); } else { @@ -204,8 +202,8 @@ private String acquireAuthCode(@NotNull final String resource, String userDispla } String message = code.getError() - + (code.getErrorDescription() == null ? "" : ": ") - + code.getErrorDescription(); + + (code.getErrorDescription() == null ? "" : ": ") + + code.getErrorDescription(); log.log(Level.SEVERE, message); throw new AuthException(code.getError(), code.getErrorDescription()); } @@ -220,7 +218,8 @@ private AuthResult acquireTokenFromCache(@NotNull String resource, String userId } private AuthResult getTokenWithAuthCode(@NotNull final String code, - @NotNull final String resource) throws AuthException { + @NotNull final String resource, + final String redirectUrl) throws AuthException { Map requestParameters = new HashMap<>(); requestParameters.put(OAuthParameter.Resource, resource); requestParameters.put(OAuthParameter.ClientId, clientId); diff --git a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/AdAuthManager.java b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/AdAuthManager.java index c71d9979c2a..e3a323bcd71 100644 --- a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/AdAuthManager.java +++ b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/AdAuthManager.java @@ -24,6 +24,7 @@ import com.microsoft.azure.management.resources.Subscription; import com.microsoft.azure.management.resources.Tenant; +import com.microsoft.azuretools.Constants; import com.microsoft.azuretools.adauth.*; import com.microsoft.azuretools.authmanage.models.AdAuthDetails; import com.microsoft.azuretools.authmanage.models.AuthMethodDetails; @@ -66,10 +67,11 @@ public String getAccessToken(String tid) throws IOException { * @throws IOException thrown when fail to get access token. */ public String getAccessToken(String tid, String resource, PromptBehavior promptBehavior) throws IOException { - AuthContext ac = createContext(tid, null, this.webUi); + AuthContext ac = createContext(tid, null); AuthResult result = null; try { - result = ac.acquireToken(resource, false, adAuthDetails.getAccountEmail(), false); + result = ac.acquireToken(resource, false, adAuthDetails.getAccountEmail(), false, + this.webUi, Constants.redirectUri); } catch (AuthException e) { if (AuthError.InvalidGrant.equalsIgnoreCase(e.getError()) || AuthError.InteractionRequired.equalsIgnoreCase(e.getError())) { @@ -142,10 +144,11 @@ public AuthResult signIn(@Nullable AuthResult savedAuth) throws IOException { if (savedAuth == null) { cleanCache(); - AuthContext ac = createContext(getCommonTenantId(), null, this.webUi); + AuthContext ac = createContext(getCommonTenantId(), null); // todo: to determine which acquireToken to call, device login or interactive login // todo: https://github.com/Microsoft/azure-tools-for-java/pull/1623 - result = ac.acquireToken(env.managementEndpoint(), true, null, false); + result = ac.acquireToken(env.managementEndpoint(), true, null, + false, this.webUi, Constants.redirectUri); } else { result = savedAuth; } @@ -158,19 +161,22 @@ public AuthResult signIn(@Nullable AuthResult savedAuth) throws IOException { List tenants = AccessTokenAzureManager.getTenants(getCommonTenantId()); for (Tenant t : tenants) { String tid = t.tenantId(); - AuthContext ac1 = createContext(tid, null, this.webUi); + AuthContext ac1 = createContext(tid, null); // put tokens into the cache try { - ac1.acquireToken(env.managementEndpoint(), false, userId, isDisplayable); + ac1.acquireToken(env.managementEndpoint(), false, userId, isDisplayable, + this.webUi, Constants.redirectUri); } catch (AuthException e) { //TODO: should narrow to AuthError.InteractionRequired - ac1.acquireToken(env.managementEndpoint(), true, userId, isDisplayable); + ac1.acquireToken(env.managementEndpoint(), true, userId, isDisplayable, + this.webUi, Constants.redirectUri); } // FIXME!!! Some environments and subscriptions can't get the resource manager token // Let the log in process passed, and throwing the errors when to access those resources try { - ac1.acquireToken(env.resourceManagerEndpoint(), false, userId, isDisplayable); + ac1.acquireToken(env.resourceManagerEndpoint(), false, userId, isDisplayable, + this.webUi, Constants.redirectUri); } catch (AuthException e) { if (CommonSettings.getEnvironment() instanceof ProvidedEnvironment) { // Swallow the exception since some provided environments are not full featured @@ -180,7 +186,8 @@ public AuthResult signIn(@Nullable AuthResult savedAuth) throws IOException { } try { - ac1.acquireToken(env.graphEndpoint(), false, userId, isDisplayable); + ac1.acquireToken(env.graphEndpoint(), false, userId, isDisplayable, + this.webUi, Constants.redirectUri); } catch (AuthException e) { if (CommonSettings.getEnvironment() instanceof ProvidedEnvironment) { // Swallow the exception since some provided environments are not full featured @@ -191,7 +198,8 @@ public AuthResult signIn(@Nullable AuthResult savedAuth) throws IOException { // ADL account access token try { - ac1.acquireToken(env.dataLakeEndpointResourceId(), false, userId, isDisplayable); + ac1.acquireToken(env.dataLakeEndpointResourceId(), false, userId, isDisplayable, + this.webUi, Constants.redirectUri); } catch (AuthException e) { LOGGER.warning("Can't get " + env.dataLakeEndpointResourceId() + " access token from environment " + CommonSettings.getEnvironment().getName() + "for user " + userId); @@ -228,24 +236,19 @@ private AuthResult loadFromSecureStore() { return null; } - String authJson = secureStore.loadPassword(SECURE_STORE_SERVICE, SECURE_STORE_KEY); - + final String authJson = secureStore.loadPassword(SECURE_STORE_SERVICE, SECURE_STORE_KEY); if (authJson != null) { try { - AuthResult savedAuth = JsonHelper.deserialize(AuthResult.class, authJson); - + final AuthResult savedAuth = JsonHelper.deserialize(AuthResult.class, authJson); if (!savedAuth.getUserId().equals(adAuthDetails.getAccountEmail())) { return null; } - String tenantId = StringUtils.isNullOrWhiteSpace(savedAuth.getUserInfo().getTenantId()) ? COMMON_TID : - savedAuth.getUserInfo().getTenantId(); - - AuthContext ac = createContext(tenantId, null, this.webUi); - AuthResult updatedAuth = ac.acquireToken(savedAuth); - + final String tenantId = StringUtils.isNullOrWhiteSpace(savedAuth.getUserInfo().getTenantId()) ? + COMMON_TID : savedAuth.getUserInfo().getTenantId(); + final AuthContext ac = createContext(tenantId, null); + final AuthResult updatedAuth = ac.acquireToken(savedAuth); saveToSecureStore(updatedAuth); - return updatedAuth; } catch (IOException e) { LOGGER.warning("Can't restore the authentication cache: " + e.getMessage()); diff --git a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/BaseADAuthManager.java b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/BaseADAuthManager.java index fd4f98685e9..32450c158f7 100644 --- a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/BaseADAuthManager.java +++ b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/BaseADAuthManager.java @@ -119,9 +119,7 @@ protected void saveToSecureStore(@Nullable AuthResult authResult) { } } - // todo: webUi and redirectUri is only used by interactive login, they should not be the properties of authcontext - protected AuthContext createContext(@NotNull final String tid, final UUID corrId, - final IWebUi webUi) throws IOException { + protected AuthContext createContext(@NotNull final String tid, final UUID corrId) throws IOException { String authority = null; final String endpoint = env.activeDirectoryEndpoint(); if (StringUtils.isNullOrEmpty(endpoint)) { @@ -132,6 +130,6 @@ protected AuthContext createContext(@NotNull final String tid, final UUID corrId } else { authority = endpoint + "/" + tid; } - return new AuthContext(authority, Constants.clientId, Constants.redirectUri, webUi, true, corrId); + return new AuthContext(authority, Constants.clientId, true, corrId); } } diff --git a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/DCAuthManager.java b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/DCAuthManager.java index b719a1b1a05..06ed79a76ff 100644 --- a/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/DCAuthManager.java +++ b/Utils/azuretools-core/src/com/microsoft/azuretools/authmanage/DCAuthManager.java @@ -44,7 +44,7 @@ public static DCAuthManager getInstance() { public AuthResult deviceLogin(final AuthenticationCallback callback) throws IOException { cleanCache(); - final AuthContext ac = createContext(getCommonTenantId(), null, null); + final AuthContext ac = createContext(getCommonTenantId(), null); final AuthResult result = ac.acquireToken(env.managementEndpoint(), true, null, callback); if (!result.isUserIdDisplayble()) { // todo refactor the words