Skip to content

Commit

Permalink
MSI Refactoring (#715)
Browse files Browse the repository at this point in the history
* Refactor the Application class hierarchy to better separate Public/ConfidentialClientApplication and ManagedIdentityApplication

* Refactor ManagedIdentity classes to better follow Java best-practices, reduce use of public scope, and generally match the library's existing code styles

* Merge cloud shell changes

* Address code review comments

* Move instance discovery and region fields back into AbstractClientApplicationBase only

* Better grouping of public vs. non-public APIs
  • Loading branch information
Avery-Dunn authored Oct 5, 2023
1 parent 5f5e32e commit 450757a
Show file tree
Hide file tree
Showing 34 changed files with 573 additions and 471 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,35 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
ServiceBundle serviceBundle) {
String host = authorityUrl.getHost();

if (shouldUseRegionalEndpoint(msalRequest)) {
if (msalRequest.application() instanceof AbstractClientApplicationBase && shouldUseRegionalEndpoint(msalRequest)) {
//Server side telemetry requires the result from region discovery when any part of the region API is used
String detectedRegion = discoverRegion(msalRequest, serviceBundle);

if (msalRequest.application().azureRegion() != null) {
host = getRegionalizedHost(authorityUrl.getHost(), msalRequest.application().azureRegion());
if (((AbstractClientApplicationBase) msalRequest.application()).azureRegion() != null) {
host = getRegionalizedHost(authorityUrl.getHost(),
((AbstractClientApplicationBase) msalRequest.application()).azureRegion());
}

//If region autodetection is enabled and a specific region not already set,
// set the application's region to the discovered region so that future requests can skip the IMDS endpoint call
if (null == msalRequest.application().azureRegion() && msalRequest.application().autoDetectRegion()
if (null == ((AbstractClientApplicationBase) msalRequest.application()).azureRegion()
&& ((AbstractClientApplicationBase) msalRequest.application()).autoDetectRegion()
&& null != detectedRegion) {
msalRequest.application().azureRegion = detectedRegion;
((AbstractClientApplicationBase) msalRequest.application()).azureRegion = detectedRegion;
}
cacheRegionInstanceMetadata(authorityUrl.getHost(), msalRequest.application().azureRegion());
cacheRegionInstanceMetadata(authorityUrl.getHost(), ((AbstractClientApplicationBase) msalRequest.application()).azureRegion());
serviceBundle.getServerSideTelemetry().getCurrentRequest().regionOutcome(
determineRegionOutcome(detectedRegion, msalRequest.application().azureRegion(), msalRequest.application().autoDetectRegion()));
determineRegionOutcome(detectedRegion,
((AbstractClientApplicationBase) msalRequest.application()).azureRegion(),
((AbstractClientApplicationBase) msalRequest.application()).autoDetectRegion()));
}

InstanceDiscoveryMetadataEntry result = cache.get(host);

if (result == null) {
if(msalRequest.application().instanceDiscovery() && !instanceDiscoveryFailed){
if(msalRequest.application() instanceof AbstractClientApplicationBase &&
((AbstractClientApplicationBase) msalRequest.application()).instanceDiscovery()
&& !instanceDiscoveryFailed){
doInstanceDiscoveryAndCache(authorityUrl, validateAuthority, msalRequest, serviceBundle);
} else {
// instanceDiscovery flag is set to False. Do not perform instanceDiscovery.
Expand Down Expand Up @@ -145,7 +151,8 @@ static void cacheInstanceDiscoveryMetadata(String host,


private static boolean shouldUseRegionalEndpoint(MsalRequest msalRequest){
if (msalRequest.application().azureRegion() != null || msalRequest.application().autoDetectRegion()){
if (((AbstractClientApplicationBase) msalRequest.application()).azureRegion() != null
|| ((AbstractClientApplicationBase) msalRequest.application()).autoDetectRegion()){
//This class type check is a quick and dirty fix to accommodate changes to the internal workings of the region API
//
//ESTS-R only supports a small, but growing, number of scenarios, and the original design failed silently whenever
Expand Down
Loading

0 comments on commit 450757a

Please sign in to comment.