Skip to content

Commit c475312

Browse files
Avery-DunnAvery-Dunn
authored
Merge pull request #1018 from AzureAD/avdunn/instance-discovery-improvement
Improve sovereign cloud support and network error handling
2 parents 3ba5a85 + 0112055 commit c475312

File tree

5 files changed

+562
-4
lines changed

5 files changed

+562
-4
lines changed

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,14 @@ class AadInstanceDiscoveryProvider {
4343
static {
4444
TRUSTED_SOVEREIGN_HOSTS_SET.addAll(Arrays.asList(
4545
"login.chinacloudapi.cn",
46+
"login.partner.microsoftonline.cn",
4647
"login-us.microsoftonline.com",
4748
"login.microsoftonline.de",
48-
"login.microsoftonline.us"));
49+
"login.microsoftonline.us",
50+
"login.usgovcloudapi.net",
51+
"login.sovcloud-identity.fr",
52+
"login.sovcloud-identity.de",
53+
"login.sovcloud-identity.sg"));
4954

5055
TRUSTED_HOSTS_SET.addAll(Arrays.asList(
5156
DEFAULT_TRUSTED_HOST,
@@ -142,7 +147,14 @@ static void cacheInstanceDiscoveryResponse(String host,
142147
}
143148

144149
static void cacheInstanceDiscoveryMetadata(String host) {
145-
cache.putIfAbsent(host, new InstanceDiscoveryMetadataEntry(host, host, Collections.singleton(host)));
150+
InstanceDiscoveryMetadataEntry knownEntry = KnownMetadataProvider.getMetadataEntry(host);
151+
if (knownEntry != null) {
152+
for (String alias : knownEntry.aliases()) {
153+
cache.putIfAbsent(alias, knownEntry);
154+
}
155+
} else {
156+
cache.putIfAbsent(host, new InstanceDiscoveryMetadataEntry(host, host, Collections.singleton(host)));
157+
}
146158
}
147159

148160
private static boolean shouldUseRegionalEndpoint(MsalRequest msalRequest){
@@ -234,7 +246,7 @@ static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL authorityUr
234246
AadInstanceDiscoveryResponse response = JsonHelper.convertJsonStringToJsonSerializableObject(httpResponse.body(), AadInstanceDiscoveryResponse::fromJson);
235247

236248
if (httpResponse.statusCode() != HttpStatus.HTTP_OK) {
237-
if (httpResponse.statusCode() == HttpStatus.HTTP_BAD_REQUEST && response.error().equals("invalid_instance")) {
249+
if (httpResponse.statusCode() == HttpStatus.HTTP_BAD_REQUEST && response.error().equals(AuthenticationErrorCode.INVALID_INSTANCE)) {
238250
// instance discovery failed due to an invalid authority, throw an exception.
239251
throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse);
240252
}
@@ -340,7 +352,26 @@ private static void doInstanceDiscoveryAndCache(URL authorityUrl,
340352
AadInstanceDiscoveryResponse aadInstanceDiscoveryResponse = null;
341353

342354
if (msalRequest.application().authenticationAuthority.authorityType.equals(AuthorityType.AAD)) {
343-
aadInstanceDiscoveryResponse = sendInstanceDiscoveryRequest(authorityUrl, msalRequest, serviceBundle);
355+
try {
356+
aadInstanceDiscoveryResponse = sendInstanceDiscoveryRequest(authorityUrl, msalRequest, serviceBundle);
357+
} catch (MsalServiceException ex) {
358+
// Throw "invalid_instance" errors: this means the authority itself is invalid.
359+
// All other HTTP-level errors (500, 502, 404, etc.) should fall through to the fallback path.
360+
if (ex.errorCode().equals(AuthenticationErrorCode.INVALID_INSTANCE)) {
361+
throw ex;
362+
}
363+
LOG.warn("Instance discovery request failed with a service error. " +
364+
"MSAL will use fallback instance metadata for {}. Error: {}", authorityUrl.getHost(), ex.getMessage());
365+
cacheInstanceDiscoveryMetadata(authorityUrl.getHost());
366+
return;
367+
} catch (Exception e) {
368+
// Network failures (timeout, DNS, connection refused) — cache a fallback
369+
// entry so subsequent calls don't retry the failing network call.
370+
LOG.warn("Instance discovery network request failed. " +
371+
"MSAL will use fallback instance metadata for {}. Exception: {}", authorityUrl.getHost(), e.getMessage());
372+
cacheInstanceDiscoveryMetadata(authorityUrl.getHost());
373+
return;
374+
}
344375

345376
if (validateAuthority) {
346377
validate(aadInstanceDiscoveryResponse);

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,10 @@ public class AuthenticationErrorCode {
160160
public static final String CRYPTO_ERROR = "crypto_error";
161161

162162
public static final String INVALID_TIMESTAMP_FORMAT = "invalid_timestamp_format";
163+
164+
/**
165+
* Indicates that instance discovery failed because the authority is not a valid instance.
166+
* This is returned by the instance discovery endpoint when the provided authority host is unknown.
167+
*/
168+
public static final String INVALID_INSTANCE = "invalid_instance";
163169
}

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/KnownMetadataProvider.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.aad.msal4j;
5+
6+
import java.util.*;
7+
8+
/**
9+
* Provides hardcoded instance discovery metadata for well-known cloud environments.
10+
* This allows correct alias resolution and cache behavior even when the network
11+
* instance discovery endpoint is unreachable.
12+
*
13+
* Mirrors the KnownMetadataProvider in MSAL .NET.
14+
*/
15+
class KnownMetadataProvider {
16+
17+
private static final Map<String, InstanceDiscoveryMetadataEntry> KNOWN_ENTRIES;
18+
19+
static {
20+
Map<String, InstanceDiscoveryMetadataEntry> entries = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
21+
22+
addEntry(entries,
23+
"login.microsoftonline.com", "login.windows.net",
24+
"login.microsoftonline.com", "login.windows.net", "login.microsoft.com", "sts.windows.net");
25+
26+
addEntry(entries,
27+
"login.partner.microsoftonline.cn", "login.partner.microsoftonline.cn",
28+
"login.partner.microsoftonline.cn", "login.chinacloudapi.cn");
29+
30+
addEntry(entries,
31+
"login.microsoftonline.de", "login.microsoftonline.de",
32+
"login.microsoftonline.de");
33+
34+
addEntry(entries,
35+
"login.microsoftonline.us", "login.microsoftonline.us",
36+
"login.microsoftonline.us", "login.usgovcloudapi.net");
37+
38+
addEntry(entries,
39+
"login-us.microsoftonline.com", "login-us.microsoftonline.com",
40+
"login-us.microsoftonline.com");
41+
42+
addEntry(entries,
43+
"login.sovcloud-identity.fr", "login.sovcloud-identity.fr",
44+
"login.sovcloud-identity.fr");
45+
46+
addEntry(entries,
47+
"login.sovcloud-identity.de", "login.sovcloud-identity.de",
48+
"login.sovcloud-identity.de");
49+
50+
addEntry(entries,
51+
"login.sovcloud-identity.sg", "login.sovcloud-identity.sg",
52+
"login.sovcloud-identity.sg");
53+
54+
KNOWN_ENTRIES = Collections.unmodifiableMap(entries);
55+
}
56+
57+
private static void addEntry(Map<String, InstanceDiscoveryMetadataEntry> entries,
58+
String preferredNetwork,
59+
String preferredCache,
60+
String... aliases) {
61+
Set<String> aliasSet = new LinkedHashSet<>(Arrays.asList(aliases));
62+
InstanceDiscoveryMetadataEntry entry = new InstanceDiscoveryMetadataEntry(preferredNetwork, preferredCache, aliasSet);
63+
for (String alias : aliases) {
64+
entries.put(alias, entry);
65+
}
66+
}
67+
68+
/**
69+
* Returns the known metadata entry for the given host, or null if unknown.
70+
*/
71+
static InstanceDiscoveryMetadataEntry getMetadataEntry(String host) {
72+
return KNOWN_ENTRIES.get(host);
73+
}
74+
75+
/**
76+
* Returns true if the host is a well-known cloud environment.
77+
*/
78+
static boolean isKnownEnvironment(String host) {
79+
return KNOWN_ENTRIES.containsKey(host);
80+
}
81+
}

‎msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/KnownMetadataProviderTest.java‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.aad.msal4j;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
import static org.junit.jupiter.api.Assertions.*;
9+
10+
class KnownMetadataProviderTest {
11+
12+
@Test
13+
void isKnownEnvironment_allKnownHosts_returnsTrue() {
14+
String[] knownHosts = {
15+
"login.microsoftonline.com", "login.windows.net", "login.microsoft.com", "sts.windows.net",
16+
"login.partner.microsoftonline.cn", "login.chinacloudapi.cn",
17+
"login.microsoftonline.de",
18+
"login.microsoftonline.us", "login.usgovcloudapi.net",
19+
"login-us.microsoftonline.com",
20+
"login.sovcloud-identity.fr", "login.sovcloud-identity.de", "login.sovcloud-identity.sg"
21+
};
22+
for (String host : knownHosts) {
23+
assertTrue(KnownMetadataProvider.isKnownEnvironment(host),
24+
"Expected " + host + " to be a known environment");
25+
}
26+
}
27+
28+
@Test
29+
void isKnownEnvironment_unknownHost_returnsFalse() {
30+
assertFalse(KnownMetadataProvider.isKnownEnvironment("custom.authority.example.com"));
31+
}
32+
33+
@Test
34+
void getMetadataEntry_publicCloud_returnsCorrectAliases() {
35+
// Arrange / Act
36+
InstanceDiscoveryMetadataEntry entry = KnownMetadataProvider.getMetadataEntry("login.microsoftonline.com");
37+
38+
// Assert
39+
assertNotNull(entry);
40+
assertEquals("login.microsoftonline.com", entry.preferredNetwork());
41+
assertEquals("login.windows.net", entry.preferredCache());
42+
assertEquals(4, entry.aliases().size());
43+
assertTrue(entry.aliases().contains("login.microsoftonline.com"));
44+
assertTrue(entry.aliases().contains("login.windows.net"));
45+
assertTrue(entry.aliases().contains("login.microsoft.com"));
46+
assertTrue(entry.aliases().contains("sts.windows.net"));
47+
}
48+
49+
@Test
50+
void getMetadataEntry_publicCloudAliases_resolvesToSameEntry() {
51+
// All public cloud aliases should resolve to the same object
52+
InstanceDiscoveryMetadataEntry entry1 = KnownMetadataProvider.getMetadataEntry("login.microsoftonline.com");
53+
InstanceDiscoveryMetadataEntry entry2 = KnownMetadataProvider.getMetadataEntry("login.windows.net");
54+
InstanceDiscoveryMetadataEntry entry3 = KnownMetadataProvider.getMetadataEntry("login.microsoft.com");
55+
InstanceDiscoveryMetadataEntry entry4 = KnownMetadataProvider.getMetadataEntry("sts.windows.net");
56+
57+
// Assert
58+
assertSame(entry1, entry2);
59+
assertSame(entry2, entry3);
60+
assertSame(entry3, entry4);
61+
}
62+
63+
@Test
64+
void getMetadataEntry_unknownHost_returnsNull() {
65+
assertNull(KnownMetadataProvider.getMetadataEntry("custom.authority.example.com"));
66+
}
67+
68+
@Test
69+
void getMetadataEntry_caseInsensitive() {
70+
// Assert — lookups should be case-insensitive
71+
assertNotNull(KnownMetadataProvider.getMetadataEntry("LOGIN.MICROSOFTONLINE.COM"));
72+
assertNotNull(KnownMetadataProvider.getMetadataEntry("Login.Partner.Microsoftonline.Cn"));
73+
assertNotNull(KnownMetadataProvider.getMetadataEntry("LOGIN.SOVCLOUD-IDENTITY.FR"));
74+
}
75+
76+
@Test
77+
void cacheInstanceDiscoveryMetadata_knownHost_cachesAllAliases() {
78+
// Arrange
79+
AadInstanceDiscoveryProvider.cache.clear();
80+
81+
// Act
82+
AadInstanceDiscoveryProvider.cacheInstanceDiscoveryMetadata("login.microsoftonline.com");
83+
84+
// Assert — all public cloud aliases should be cached
85+
assertNotNull(AadInstanceDiscoveryProvider.cache.get("login.microsoftonline.com"));
86+
assertNotNull(AadInstanceDiscoveryProvider.cache.get("login.windows.net"));
87+
assertNotNull(AadInstanceDiscoveryProvider.cache.get("login.microsoft.com"));
88+
assertNotNull(AadInstanceDiscoveryProvider.cache.get("sts.windows.net"));
89+
90+
// All should reference the same entry
91+
InstanceDiscoveryMetadataEntry entry = AadInstanceDiscoveryProvider.cache.get("login.microsoftonline.com");
92+
assertSame(entry, AadInstanceDiscoveryProvider.cache.get("login.windows.net"));
93+
assertSame(entry, AadInstanceDiscoveryProvider.cache.get("login.microsoft.com"));
94+
assertSame(entry, AadInstanceDiscoveryProvider.cache.get("sts.windows.net"));
95+
}
96+
97+
@Test
98+
void cacheInstanceDiscoveryMetadata_unknownHost_cachesSelfEntry() {
99+
// Arrange
100+
AadInstanceDiscoveryProvider.cache.clear();
101+
102+
// Act
103+
AadInstanceDiscoveryProvider.cacheInstanceDiscoveryMetadata("custom.unknown.example.com");
104+
105+
// Assert — should get a self-referencing entry
106+
InstanceDiscoveryMetadataEntry entry = AadInstanceDiscoveryProvider.cache.get("custom.unknown.example.com");
107+
assertNotNull(entry);
108+
assertEquals("custom.unknown.example.com", entry.preferredNetwork());
109+
assertEquals("custom.unknown.example.com", entry.preferredCache());
110+
assertEquals(1, entry.aliases().size());
111+
assertTrue(entry.aliases().contains("custom.unknown.example.com"));
112+
}
113+
}

0 commit comments

Comments
 (0)