From 12ca83641b0f1165787b85ec5206812cbcb8c55b Mon Sep 17 00:00:00 2001 From: siddhijain Date: Tue, 7 Mar 2023 17:08:01 -0600 Subject: [PATCH 1/2] update condition to throw exception --- .../aad/msal4j/AadInstanceDiscoveryProvider.java | 11 +++++++++-- .../java/com/microsoft/aad/msal4j/HttpHelper.java | 3 +++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java index b4d61b27..1519c98f 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java +++ b/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 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. @@ -235,7 +237,12 @@ private static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL aut httpResponse = executeRequest(instanceDiscoveryRequestUrl, msalRequest.headers().getReadonlyHeaderMap(), msalRequest, serviceBundle); if (httpResponse.statusCode() != HttpHelper.HTTP_STATUS_200) { - throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse); + if(httpResponse.statusCode() == HttpHelper.HTTP_STATUS_400 && httpResponse.body().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; } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java index 2c088fd5..cc6b4e7d 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java +++ b/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; From cf814eb0f30967aae90dd462374c60237ad63423 Mon Sep 17 00:00:00 2001 From: siddhijain Date: Thu, 9 Mar 2023 11:55:59 -0600 Subject: [PATCH 2/2] added test for invalid authority --- .../InvalidAuthorityIT.java | 26 +++++++++++++++++++ .../msal4j/AadInstanceDiscoveryProvider.java | 7 ++--- 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/InvalidAuthorityIT.java diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/InvalidAuthorityIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/InvalidAuthorityIT.java new file mode 100644 index 00000000..07be1538 --- /dev/null +++ b/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 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(); + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java index 1519c98f..a66094e9 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java @@ -236,8 +236,10 @@ 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) { - if(httpResponse.statusCode() == HttpHelper.HTTP_STATUS_400 && httpResponse.body().equals("invalid_instance")){ + 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); } @@ -245,8 +247,7 @@ private static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL aut instanceDiscoveryFailed = true; } - - return JsonHelper.convertJsonToObject(httpResponse.body(), AadInstanceDiscoveryResponse.class); + return response; } private static int determineRegionOutcome(String detectedRegion, String providedRegion, boolean autoDetect) {