Merge pull request #606 from AzureAD/SJAIN/Issue-605

Instance discovery throws exception only with invalid authority

Author: siddhijain

msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/InvalidAuthorityIT.java

@@ -0,0 +1,26 @@
+package com.microsoft.aad.msal4j;
+
+import org.testng.annotations.Test;
+
+import java.net.URI;
+import java.util.Collections;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+
+public class InvalidAuthorityIT extends SeleniumTest{
+
+    @Test(expectedExceptions = ExecutionException.class, expectedExceptionsMessageRegExp = ".*?invalid instance.*?")
+    public void acquireTokenWithAuthorizationCode_InvalidAuthority() throws Exception{
+        PublicClientApplication app;
+        app = PublicClientApplication.builder(
+                        TestConfiguration.AAD_CLIENT_ID)
+                .authority("https://dummy.microsoft.com/common") //invalid authority, request fails at instance discovery
+                .build();
+
+        CompletableFuture<IAuthenticationResult> future = app.acquireToken(
+                AuthorizationCodeParameters.builder("auth_code", new URI(TestConfiguration.AAD_DEFAULT_REDIRECT_URI))
+                        .scopes(Collections.singleton("default-scope"))
+                        .authorizationCode("auth_code").redirectUri(new URI(TestConfiguration.AAD_DEFAULT_REDIRECT_URI)).build());
+        future.get();
+    }
+}

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java

@@ -38,6 +38,8 @@ class AadInstanceDiscoveryProvider {
 
     private static final Logger log = LoggerFactory.getLogger(AadInstanceDiscoveryProvider.class);
 
+    //flag to check if instance discovery has failed
+    private static boolean instanceDiscoveryFailed = false;
     static ConcurrentHashMap<String, InstanceDiscoveryMetadataEntry> cache = new ConcurrentHashMap<>();
 
     static {
@@ -84,7 +86,7 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
         InstanceDiscoveryMetadataEntry result = cache.get(host);
 
         if (result == null) {
-            if(msalRequest.application().instanceDiscovery()){
+            if(msalRequest.application().instanceDiscovery() && !instanceDiscoveryFailed){
                 doInstanceDiscoveryAndCache(authorityUrl, validateAuthority, msalRequest, serviceBundle);
             } else {
                 // instanceDiscovery flag is set to False. Do not perform instanceDiscovery.
@@ -234,12 +236,18 @@ private static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL aut
 
         httpResponse = executeRequest(instanceDiscoveryRequestUrl, msalRequest.headers().getReadonlyHeaderMap(), msalRequest, serviceBundle);
 
+        AadInstanceDiscoveryResponse response = JsonHelper.convertJsonToObject(httpResponse.body(), AadInstanceDiscoveryResponse.class);
+
         if (httpResponse.statusCode() != HttpHelper.HTTP_STATUS_200) {
-            throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse);
+            if(httpResponse.statusCode() == HttpHelper.HTTP_STATUS_400 && response.error().equals("invalid_instance")){
+                // instance discovery failed due to an invalid authority, throw an exception.
+                throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse);
+            }
+            // instance discovery failed due to reasons other than an invalid authority, do not perform instance discovery again in this environment.
+            instanceDiscoveryFailed = true;
         }
 
-
-        return JsonHelper.convertJsonToObject(httpResponse.body(), AadInstanceDiscoveryResponse.class);
+        return response;
     }
 
     private static int determineRegionOutcome(String detectedRegion, String providedRegion, boolean autoDetect) {

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java

@@ -22,6 +22,9 @@ class HttpHelper {
     public static final int RETRY_DELAY_MS = 1000;
 
     public static final int HTTP_STATUS_200 = 200;
+
+    public static final int HTTP_STATUS_400 = 400;
+
     public static final int HTTP_STATUS_429 = 429;
     public static final int HTTP_STATUS_500 = 500;