Fix ABBA deadlock between AzureActiveDirectory and AzureActiveDirectoryAuthority

- Extract authority.getAuthorityURL() outside synchronized(sLock) in
  Authority.isKnownAuthority() to avoid lock-ordering inversion
- Extract authority.getAuthorityURL() outside synchronized scope in
  AzureActiveDirectory.ensureCloudDiscoveryForAuthority(Authority)
- Remove synchronized from read-only ConcurrentHashMap methods in
  AzureActiveDirectory (hasCloudHost, getAzureActiveDirectoryCloud,
  getAzureActiveDirectoryCloudFromHostName, isValidCloudHost)
- Remove synchronized from AzureActiveDirectoryAuthority.getAzureActiveDirectoryCloud()
- Add null check for authorityUrl in isKnownAuthority()
- Improve error handling in getKnownAuthorityResult(): propagate original
  discovery exception instead of replacing with UNKNOWN_AUTHORITY
- Make KnownAuthorityResult fields final
- Add 5 concurrency regression tests in AzureActiveDirectoryTest
- Expand AuthorityKnownAuthorityTest with sovereign cloud, error propagation,
  and mock-based getKnownAuthorityResult tests

AB#3578299

Author: Mohit

common4j/src/main/com/microsoft/identity/common/java/authorities/Authority.java

@@ -318,40 +318,42 @@ public static void addKnownAuthorities(List<Authority> authorities) {
     }
 
     /**
-     * Authorities are either known by the developer and communicated to the library via configuration or they
-     * are known to Microsoft based on the list of clouds returned from:
+     * Authorities are either known by the developer and communicated to the library via
+     * configuration, or they are known to Microsoft based on the cloud discovery metadata cache.
      *
      * @param authority Authority to check against.
      * @return True if the authority is known to Microsoft or defined in the configuration.
      */
     public static boolean isKnownAuthority(Authority authority) {
-        final String methodName = ":isKnownAuthority";
+        final String methodTag = TAG + ":isKnownAuthority";
         boolean knownToDeveloper = false;
         boolean knownToMicrosoft;
 
         if (authority == null) {
-            Logger.warn(
-                    TAG + methodName,
-                    "Authority is null"
-            );
+            Logger.warn(methodTag, "Authority is null");
+            return false;
+        }
+
+        // Resolve the authority URL outside any lock to avoid calling a potentially-
+        // synchronized polymorphic method (e.g. AzureActiveDirectoryAuthority.getAuthorityUri())
+        // while holding sLock, which could create a lock-ordering inversion.
+        final URL authorityUrl = authority.getAuthorityURL();
+
+        if (authorityUrl == null) {
+            Logger.warn(methodTag, "Authority URL is null");
             return false;
         }
 
-        if (BuildConfig.ALLOW_ONEBOX_AUTHORITIES && AzureActiveDirectoryEnvironment.ONEBOX_AUTHORITY.equals(authority.getAuthorityURL().getAuthority())) {
+        if (BuildConfig.ALLOW_ONEBOX_AUTHORITIES && AzureActiveDirectoryEnvironment.ONEBOX_AUTHORITY.equals(authorityUrl.getAuthority())) {
             return true; // onebox authorities are always considered to be known.
         }
 
-        //Check if authority was added to configuration
         synchronized (sLock) {
             for (final Authority currentAuthority : knownAuthorities) {
                 if (currentAuthority.mAuthorityUrlString != null &&
-                        authority.getAuthorityURL() != null &&
-                        authority.getAuthorityURL().getAuthority() != null &&
+                        authorityUrl.getAuthority() != null &&
                         currentAuthority.mAuthorityUrlString.toLowerCase(Locale.ROOT).contains(
-                                authority
-                                        .getAuthorityURL()
-                                        .getAuthority()
-                                        .toLowerCase(Locale.ROOT))) {
+                                authorityUrl.getAuthority().toLowerCase(Locale.ROOT))) {
                     knownToDeveloper = true;
                     break;
                 }
@@ -360,19 +362,14 @@ public static boolean isKnownAuthority(Authority authority) {
 
         // Check whether the authority is known to Microsoft or not.  Microsoft can recognize authorities that exist within public clouds.
         // Microsoft does not maintain a list of B2C authorities or a list of ADFS or 3rd party authorities (issuers).
-        knownToMicrosoft = AzureActiveDirectory.hasCloudHost(authority.getAuthorityURL());
+        knownToMicrosoft = AzureActiveDirectory.hasCloudHost(authorityUrl);
 
         final boolean isKnown = (knownToDeveloper || knownToMicrosoft);
 
-        Logger.verbose(
-                TAG + methodName,
-                "Authority is known to developer? [" + knownToDeveloper + "]"
-        );
-
-        Logger.verbose(
-                TAG + methodName,
-                "Authority is known to Microsoft? [" + knownToMicrosoft + "]"
-        );
+        Logger.verbose(methodTag,
+                "Authority is known to developer? [" + knownToDeveloper + "]");
+        Logger.verbose(methodTag,
+                "Authority is known to Microsoft? [" + knownToMicrosoft + "]");
 
         return isKnown;
     }
@@ -383,37 +380,43 @@ public static KnownAuthorityResult getKnownAuthorityResult(Authority authority)
                 methodTag,
                 "Getting known authority result..."
         );
-        ClientException clientException = null;
-        boolean known = false;
 
+        ClientException discoveryException = null;
         try {
             AzureActiveDirectory.ensureCloudDiscoveryForAuthority(authority);
             Logger.info(methodTag, "Cloud discovery complete.");
         } catch (final ClientException ex) {
             // Cloud discovery failed (e.g. network error).
             // Log but continue — the authority may still be known via hardcoded
             // metadata or developer configuration.
+            discoveryException = ex;
             Logger.warn(methodTag,
                     "Cloud discovery failed, will check hardcoded/configured authorities. Error: "
                             + ex.getErrorCode());
         }
 
-        if (!isKnownAuthority(authority)) {
-            clientException = new ClientException(
+        if (isKnownAuthority(authority)) {
+            Logger.info(methodTag, "Authority is known.");
+            return new KnownAuthorityResult(true, null);
+        }
+
+        // Authority is not known via any source.
+        if (discoveryException != null) {
+            // Discovery failed — propagate the original error (keeping the original behavior that was before PR#3027)
+            return new KnownAuthorityResult(false, discoveryException);
+        } else {
+            // Discovery succeeded but authority is not in any known list.
+            final ClientException clientException = new ClientException(
                     ClientException.UNKNOWN_AUTHORITY,
                     "Provided authority is not known.  MSAL will only make requests to known authorities"
             );
-        } else {
-            Logger.info(methodTag, "Cloud is known.");
-            known = true;
+            return new KnownAuthorityResult(false, clientException);
         }
-
-        return new KnownAuthorityResult(known, clientException);
     }
 
     public static class KnownAuthorityResult {
-        private boolean mKnown;
-        private ClientException mClientException;
+        private final boolean mKnown;
+        private final ClientException mClientException;
 
         KnownAuthorityResult(boolean known, ClientException exception) {
             mKnown = known;

common4j/src/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java

@@ -71,7 +71,7 @@ public class AzureActiveDirectoryAuthority extends Authority {
 
     /** Gets {@link AzureActiveDirectoryCloud}, if the cloud metadata is already initialized. */
     @Nullable
-    private static synchronized AzureActiveDirectoryCloud getAzureActiveDirectoryCloud(
+    private static AzureActiveDirectoryCloud getAzureActiveDirectoryCloud(
             @NonNull final AzureActiveDirectoryAudience audience) {
         final String methodName = ":getAzureActiveDirectoryCloud";
 

common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java

@@ -164,11 +164,11 @@ public AzureActiveDirectoryOAuth2Strategy createOAuth2Strategy(@NonNull final Az
         return new AzureActiveDirectoryOAuth2Strategy(config, parameters);
     }
 
-    public static synchronized boolean hasCloudHost(@NonNull final URL authorityUrl) {
+    public static boolean hasCloudHost(@NonNull final URL authorityUrl) {
         return sAadClouds.containsKey(authorityUrl.getHost().toLowerCase(Locale.US));
     }
 
-    public static synchronized boolean isValidCloudHost(@NonNull final URL authorityUrl) {
+    public static boolean isValidCloudHost(@NonNull final URL authorityUrl) {
         return hasCloudHost(authorityUrl) && getAzureActiveDirectoryCloud(authorityUrl).isValidated();
     }
 
@@ -191,15 +191,15 @@ public static synchronized Environment getEnvironment() {
      * @param authorityUrl URL
      * @return AzureActiveDirectoryCloud
      */
-    public static synchronized AzureActiveDirectoryCloud getAzureActiveDirectoryCloud(@NonNull final URL authorityUrl) {
+    public static AzureActiveDirectoryCloud getAzureActiveDirectoryCloud(@NonNull final URL authorityUrl) {
         return sAadClouds.get(authorityUrl.getHost().toLowerCase(Locale.US));
     }
 
     /**
      * @param preferredCacheHostName String
      * @return AzureActiveDirectoryCloud
      */
-    public static synchronized AzureActiveDirectoryCloud getAzureActiveDirectoryCloudFromHostName(@NonNull final String preferredCacheHostName) {
+    public static AzureActiveDirectoryCloud getAzureActiveDirectoryCloudFromHostName(@NonNull final String preferredCacheHostName) {
         return sAadClouds.get(preferredCacheHostName.toLowerCase(Locale.US));
     }
 
@@ -368,11 +368,16 @@ public static synchronized void ensureCloudDiscovery() throws ClientException {
      *
      * @param authority The authority whose URL determines the discovery endpoint, or null to use the default.
      */
-    public static synchronized void ensureCloudDiscoveryForAuthority(@Nullable final Authority authority)
+    public static void ensureCloudDiscoveryForAuthority(@Nullable final Authority authority)
             throws ClientException {
-        ensureCloudDiscoveryForAuthority(
-                authority != null ? authority.getAuthorityURL() : null
-        );
+        // Resolve the URL outside the synchronized scope to avoid deadlock:
+        // getAuthorityURL() on AzureActiveDirectoryAuthority acquires
+        // AzureActiveDirectoryAuthority.class then AzureActiveDirectory.class,
+        // while ensureCloudDiscoveryForAuthority(URL) acquires AzureActiveDirectory.class.
+        // Calling getAuthorityURL() while holding AzureActiveDirectory.class would
+        // create a lock-ordering inversion with any concurrent getAuthorityURL() caller.
+        final URL authorityUrl = authority != null ? authority.getAuthorityURL() : null;
+        ensureCloudDiscoveryForAuthority(authorityUrl);
     }
 
     /**