Skip to content

Commit

Permalink
Don't stop trying dev credentials on failures
Browse files Browse the repository at this point in the history
Fixes Azure#34733

For our dev time credentials we want to always keep going. This change wraps any failure from the credentials in a `CredentialUnavailableException` so `ChainedTokenCredential` will continue them properly. It only does so in the context of a `ChainedTokenCredential`. Regular uses of these credentials is unaffected.
  • Loading branch information
billwert committed Jul 14, 2023
1 parent 2e5aba7 commit 33f6a77
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzureCli(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

@Override
Expand All @@ -94,7 +101,11 @@ public AccessToken getTokenSync(TokenRequestContext request) {
return accessToken;
} catch (Exception e) {
LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request, e);
throw e;
if (identityClient.getIdentityClientOptions().isChained()) {
throw new CredentialUnavailableException(e.getMessage(), e);
} else {
throw e;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzureDeveloperCli(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

@Override
Expand All @@ -94,7 +101,11 @@ public AccessToken getTokenSync(TokenRequestContext request) {
return accessToken;
} catch (Exception e) {
LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request, e);
throw e;
if (identityClient.getIdentityClientOptions().isChained()) {
throw new CredentialUnavailableException(e.getMessage(), e);
} else {
throw e;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
return identityClient.authenticateWithAzurePowerShell(request)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(), request,
error));
error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class DefaultAzureCredentialBuilder extends CredentialBuilderBase<Default
*/
public DefaultAzureCredentialBuilder() {
this.identityClientOptions.setIdentityLogOptionsImpl(new IdentityLogOptionsImpl(true));
this.identityClientOptions.setChained(true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
})
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(),
request, error));
request, error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,14 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
.map(this::updateCache)
.doOnNext(token -> LoggingUtil.logTokenSuccess(LOGGER, request))
.doOnError(error -> LoggingUtil.logTokenError(LOGGER, identityClient.getIdentityClientOptions(),
request, error));
request, error))
.onErrorMap(error -> {
if (identityClient.getIdentityClientOptions().isChained()) {
return new CredentialUnavailableException(error.getMessage(), error);
} else {
return error;
}
});
}

private AccessToken updateCache(MsalToken msalToken) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.azure.core.util.logging.ClientLogger;
import com.azure.identity.AzureAuthorityHosts;
import com.azure.identity.AuthenticationRecord;
import com.azure.identity.ChainedTokenCredential;
import com.azure.identity.TokenCachePersistenceOptions;
import com.azure.identity.implementation.util.IdentityConstants;
import com.azure.identity.implementation.util.ValidationUtil;
Expand Down Expand Up @@ -72,6 +73,8 @@ public final class IdentityClientOptions implements Cloneable {

private Duration credentialProcessTimeout = Duration.ofSeconds(10);

private boolean isChained;

/**
* Creates an instance of IdentityClientOptions with default settings.
*/
Expand Down Expand Up @@ -708,6 +711,24 @@ public void setCredentialProcessTimeout(Duration credentialProcessTimeout) {
this.credentialProcessTimeout = credentialProcessTimeout;
}

/**
* Indicates whether this options instance is part of a {@link ChainedTokenCredential}.
* @return true if this options instance is part of a {@link ChainedTokenCredential}, false otherwise.
*/
public boolean isChained() {
return this.isChained;
}

/**
* Sets whether this options instance is part of a {@link ChainedTokenCredential}.
* @param isChained
* @return the updated client options
*/
public IdentityClientOptions setChained(boolean isChained) {
this.isChained = isChained;
return this;
}

public IdentityClientOptions clone() {
IdentityClientOptions clone = new IdentityClientOptions()
.setAdditionallyAllowedTenants(this.additionallyAllowedTenants)
Expand Down

0 comments on commit 33f6a77

Please sign in to comment.