Skip to content

Commit 3a4ed35

Browse files
Avery-DunnAvery-Dunn
authored
Merge pull request #970 from AzureAD/avdunn/issuer-validation
Add OIDC issuer validation and new testing style
2 parents 946a68d + 9bb0ff6 commit 3a4ed35

File tree

5 files changed

+176
-1
lines changed

5 files changed

+176
-1
lines changed

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88

99
public class OidcAuthority extends Authority {
1010
//Part of the OpenIdConnect standard, this is appended to the authority to create the endpoint that has OIDC metadata
11-
static final String WELL_KNOWN_OPENID_CONFIGURATION = ".well-known/openid-configuration";
11+
private static final String WELL_KNOWN_OPENID_CONFIGURATION = ".well-known/openid-configuration";
1212
private static final String AUTHORITY_FORMAT = "https://%s/%s/";
13+
private static final String CIAM_AUTHORITY_FORMAT = "https://%s.ciamlogin.com/%s";
14+
15+
private String issuerFromOidcDiscovery;
1316

1417
OidcAuthority(URL authorityUrl) throws MalformedURLException {
1518
super(createOidcDiscoveryUrl(authorityUrl), AuthorityType.OIDC);
@@ -29,5 +32,52 @@ void setAuthorityProperties(OidcDiscoveryResponse instanceDiscoveryResponse) {
2932
this.tokenEndpoint = instanceDiscoveryResponse.tokenEndpoint();
3033
this.deviceCodeEndpoint = instanceDiscoveryResponse.deviceCodeEndpoint();
3134
this.selfSignedJwtAudience = this.tokenEndpoint;
35+
this.issuerFromOidcDiscovery = instanceDiscoveryResponse.issuer();
36+
37+
validateIssuer();
38+
}
39+
40+
private void validateIssuer() {
41+
if (!isIssuerValid()) {
42+
throw new MsalClientException(
43+
String.format("Invalid issuer from OIDC discovery. Issuer %s does not match authority %s, or is in an unexpected format", issuerFromOidcDiscovery, canonicalAuthorityUrl),
44+
"issuer_validation");
45+
}
46+
}
47+
48+
/**
49+
* Validates the issuer from OIDC discovery.
50+
* Issuer is valid if it matches the authority URL (without the well-known segment)
51+
* or if it follows the CIAM issuer format.
52+
*
53+
* @return true if the issuer is valid, false otherwise
54+
*/
55+
private boolean isIssuerValid() {
56+
if (issuerFromOidcDiscovery == null) {
57+
return false;
58+
}
59+
60+
// Case 1: Check against canonicalAuthorityUrl without the well-known segment
61+
String authorityWithoutWellKnown = canonicalAuthorityUrl.toString();
62+
if (authorityWithoutWellKnown.endsWith(WELL_KNOWN_OPENID_CONFIGURATION)) {
63+
authorityWithoutWellKnown = authorityWithoutWellKnown.substring(0,
64+
authorityWithoutWellKnown.length() - WELL_KNOWN_OPENID_CONFIGURATION.length());
65+
66+
// Normalize both URLs to ensure consistent comparison
67+
String normalizedAuthority = Authority.enforceTrailingSlash(authorityWithoutWellKnown);
68+
String normalizedIssuer = Authority.enforceTrailingSlash(issuerFromOidcDiscovery);
69+
70+
if (normalizedIssuer.equals(normalizedAuthority)) {
71+
return true;
72+
}
73+
}
74+
75+
// Case 2: Check CIAM format: "https://{tenant}.ciamlogin.com/{tenant}"
76+
if (!StringHelper.isNullOrBlank(tenant)) {
77+
String ciamPattern = String.format(CIAM_AUTHORITY_FORMAT, tenant, tenant);
78+
return issuerFromOidcDiscovery.startsWith(ciamPattern);
79+
}
80+
81+
return false;
3282
}
3383
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class OidcDiscoveryResponse implements JsonSerializable<OidcDiscoveryResponse> {
1515
private String authorizationEndpoint;
1616
private String tokenEndpoint;
1717
private String deviceCodeEndpoint;
18+
private String issuer;
1819

1920
public static OidcDiscoveryResponse fromJson(JsonReader jsonReader) throws IOException {
2021
OidcDiscoveryResponse response = new OidcDiscoveryResponse();
@@ -32,6 +33,9 @@ public static OidcDiscoveryResponse fromJson(JsonReader jsonReader) throws IOExc
3233
case "device_authorization_endpoint":
3334
response.deviceCodeEndpoint = reader.getString();
3435
break;
36+
case "issuer":
37+
response.issuer = reader.getString();
38+
break;
3539
default:
3640
reader.skipChildren();
3741
break;
@@ -61,4 +65,8 @@ String tokenEndpoint() {
6165
String deviceCodeEndpoint() {
6266
return this.deviceCodeEndpoint;
6367
}
68+
69+
String issuer() {
70+
return this.issuer;
71+
}
6472
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,9 @@ static void setHttpClient(IHttpClient client) {
124124
httpClient = client;
125125
httpHelper = new HttpHelper(httpClient, new ManagedIdentityRetryPolicy());
126126
}
127+
128+
static void resetHttpClient() {
129+
httpClient = new DefaultHttpClientManagedIdentity(null, null, null, null);
130+
httpHelper = new HttpHelper(httpClient, new ManagedIdentityRetryPolicy());
131+
}
127132
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ static void resetRetryPolicies() {
5252
IMDSRetryPolicy.resetToDefaults();
5353
}
5454

55+
@AfterAll
56+
static void resetServiceFabricHttpClient() {
57+
ServiceFabricManagedIdentitySource.resetHttpClient();
58+
}
59+
5560
private String getSuccessfulResponse(String resource) {
5661
long expiresOn = (System.currentTimeMillis() / 1000) + (24 * 3600);//A long-lived, 24 hour token
5762
return "{\"access_token\":\"accesstoken\",\"expires_on\":\"" + expiresOn + "\",\"resource\":\"" + resource + "\",\"token_type\":" +

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

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package com.microsoft.aad.msal4j;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.api.BeforeEach;
5+
import org.mockito.Mockito;
6+
7+
import java.net.MalformedURLException;
8+
import java.net.URL;
9+
10+
import static org.junit.jupiter.api.Assertions.*;
11+
import static org.mockito.Mockito.*;
12+
13+
class OidcAuthorityTest {
14+
15+
private OidcDiscoveryResponse mockDiscoveryResponse;
16+
17+
@BeforeEach
18+
void setup() {
19+
mockDiscoveryResponse = Mockito.mock(OidcDiscoveryResponse.class);
20+
when(mockDiscoveryResponse.authorizationEndpoint()).thenReturn("https://login.example.com/authorize");
21+
when(mockDiscoveryResponse.tokenEndpoint()).thenReturn("https://login.example.com/token");
22+
when(mockDiscoveryResponse.deviceCodeEndpoint()).thenReturn("https://login.example.com/devicecode");
23+
}
24+
25+
@Test
26+
void testSetAuthorityProperties_IssuerMatchesAuthority() throws MalformedURLException {
27+
// Arrange
28+
URL authorityUrl = new URL("https://login.example.com/tenant1/");
29+
OidcAuthority authority = new OidcAuthority(authorityUrl);
30+
31+
// Match the issuer to the authority URL (without the well-known segment)
32+
when(mockDiscoveryResponse.issuer()).thenReturn("https://login.example.com/tenant1/");
33+
34+
// Act & Assert - Should not throw exception
35+
assertDoesNotThrow(() -> authority.setAuthorityProperties(mockDiscoveryResponse));
36+
37+
// Verify properties were set
38+
assertEquals("https://login.example.com/authorize", authority.authorizationEndpoint());
39+
assertEquals("https://login.example.com/token", authority.tokenEndpoint());
40+
assertEquals("https://login.example.com/devicecode", authority.deviceCodeEndpoint());
41+
assertEquals("https://login.example.com/token", authority.selfSignedJwtAudience);
42+
}
43+
44+
@Test
45+
void testSetAuthorityProperties_IssuerFollowsCiamPattern() throws MalformedURLException {
46+
// Arrange
47+
String tenant = "contoso";
48+
URL authorityUrl = new URL("https://" + tenant + ".ciamlogin.com/" + tenant + "/");
49+
OidcAuthority authority = new OidcAuthority(authorityUrl);
50+
51+
// Set an issuer that follows CIAM pattern but doesn't exactly match the authority
52+
String ciamIssuer = "https://" + tenant + ".ciamlogin.com/" + tenant + "/v2.0";
53+
when(mockDiscoveryResponse.issuer()).thenReturn(ciamIssuer);
54+
55+
// Act & Assert - Should not throw exception
56+
assertDoesNotThrow(() -> authority.setAuthorityProperties(mockDiscoveryResponse));
57+
}
58+
59+
@Test
60+
void testSetAuthorityProperties_IssuerInvalid() throws MalformedURLException {
61+
// Arrange
62+
URL authorityUrl = new URL("https://login.example.com/tenant1/");
63+
OidcAuthority authority = new OidcAuthority(authorityUrl);
64+
65+
// Set an issuer that doesn't match the authority and doesn't follow CIAM pattern
66+
when(mockDiscoveryResponse.issuer()).thenReturn("https://login.different.com/tenant1/");
67+
68+
// Act & Assert - Should throw MsalClientException
69+
MsalClientException exception = assertThrows(MsalClientException.class,
70+
() -> authority.setAuthorityProperties(mockDiscoveryResponse));
71+
72+
// Verify exception details
73+
assertEquals("issuer_validation", exception.errorCode());
74+
assertTrue(exception.getMessage().contains("Invalid issuer from OIDC discovery"));
75+
}
76+
77+
@Test
78+
void testSetAuthorityProperties_IssuerIsNull() throws MalformedURLException {
79+
// Arrange
80+
URL authorityUrl = new URL("https://login.example.com/tenant1/");
81+
OidcAuthority authority = new OidcAuthority(authorityUrl);
82+
83+
// Set null issuer
84+
when(mockDiscoveryResponse.issuer()).thenReturn(null);
85+
86+
// Act & Assert - Should throw MsalClientException
87+
MsalClientException exception = assertThrows(MsalClientException.class,
88+
() -> authority.setAuthorityProperties(mockDiscoveryResponse));
89+
90+
// Verify exception details
91+
assertEquals("issuer_validation", exception.errorCode());
92+
assertTrue(exception.getMessage().contains("Invalid issuer from OIDC discovery"));
93+
}
94+
95+
@Test
96+
void testSetAuthorityProperties_TrailingSlashNormalization() throws MalformedURLException {
97+
// Arrange
98+
URL authorityUrl = new URL("https://login.example.com/tenant1/");
99+
OidcAuthority authority = new OidcAuthority(authorityUrl);
100+
101+
// Match the issuer to the authority but without trailing slash
102+
when(mockDiscoveryResponse.issuer()).thenReturn("https://login.example.com/tenant1");
103+
104+
// Act & Assert - Should not throw exception because normalization happens
105+
assertDoesNotThrow(() -> authority.setAuthorityProperties(mockDiscoveryResponse));
106+
}
107+
}

0 commit comments

Comments
 (0)