Skip to content

Commit dd629bf

Browse files
siddhijainsiddhijain
committed
address PR comments + more tests
1 parent b3f07e8 commit dd629bf

File tree

3 files changed

+197
-138
lines changed

3 files changed

+197
-138
lines changed

‎msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/InstanceDiscoveryTest.java‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
package com.microsoft.aad.msal4j;
2+
3+
import org.easymock.Capture;
4+
import org.easymock.EasyMock;
5+
import org.powermock.api.easymock.PowerMock;
6+
import org.powermock.core.classloader.annotations.PrepareForTest;
7+
import org.testng.Assert;
8+
import org.testng.IObjectFactory;
9+
import org.testng.annotations.DataProvider;
10+
import org.testng.annotations.ObjectFactory;
11+
import org.testng.annotations.Test;
12+
13+
import java.net.URI;
14+
import java.util.Collections;
15+
import java.util.Date;
16+
import java.util.concurrent.CompletableFuture;
17+
18+
@PrepareForTest({HttpHelper.class, PublicClientApplication.class})
19+
public class InstanceDiscoveryTest {
20+
21+
private PublicClientApplication app;
22+
23+
@ObjectFactory
24+
public IObjectFactory getObjectFactory() {
25+
return new org.powermock.modules.testng.PowerMockObjectFactory();
26+
}
27+
28+
@DataProvider(name = "aadClouds")
29+
private static Object[][] getAadClouds(){
30+
return new Object[][] {{"https://login.microsoftonline.com/common"} , // #Known to Microsoft
31+
{"https://private.cloud/foo"}//Private Cloud
32+
};
33+
}
34+
35+
@DataProvider(name = "b2cAdfsClouds")
36+
private static Object[][] getNonAadClouds(){
37+
return new Object[][] {{"https://contoso.com/adfs"},//ADFS
38+
// {"https://login.b2clogin.com/contoso/b2c_policy"}//B2C
39+
};
40+
}
41+
42+
/**
43+
* when instance_discovery flag is set to true (by default), an instance_discovery is performed for authorityType = AAD
44+
*/
45+
@Test( dataProvider = "aadClouds")
46+
public void aadInstanceDiscoveryTrue(String authority) throws Exception{
47+
app = PowerMock.createPartialMock(PublicClientApplication.class,
48+
new String[]{"acquireTokenCommon"},
49+
PublicClientApplication.builder(TestConfiguration.AAD_CLIENT_ID)
50+
.authority(authority)
51+
.instanceDiscovery(true));
52+
53+
Capture<MsalRequest> capturedMsalRequest = Capture.newInstance();
54+
55+
PowerMock.expectPrivate(app, "acquireTokenCommon",
56+
EasyMock.capture(capturedMsalRequest), EasyMock.isA(AADAuthority.class)).andReturn(
57+
AuthenticationResult.builder().
58+
accessToken("accessToken").
59+
expiresOn(new Date().getTime() + 100).
60+
refreshToken("refreshToken").
61+
idToken("idToken").environment("environment").build());
62+
63+
PowerMock.mockStatic(HttpHelper.class);
64+
65+
HttpResponse instanceDiscoveryResponse = new HttpResponse();
66+
instanceDiscoveryResponse.statusCode(200);
67+
instanceDiscoveryResponse.body(TestConfiguration.INSTANCE_DISCOVERY_RESPONSE);
68+
69+
Capture<HttpRequest> capturedHttpRequest = Capture.newInstance();
70+
71+
EasyMock.expect(
72+
HttpHelper.executeHttpRequest(
73+
EasyMock.capture(capturedHttpRequest),
74+
EasyMock.isA(RequestContext.class),
75+
EasyMock.isA(ServiceBundle.class)))
76+
.andReturn(instanceDiscoveryResponse);
77+
78+
PowerMock.replay(HttpHelper.class, HttpResponse.class);
79+
80+
CompletableFuture<IAuthenticationResult> completableFuture = app.acquireToken(
81+
AuthorizationCodeParameters.builder
82+
("auth_code",
83+
new URI(TestConfiguration.AAD_DEFAULT_REDIRECT_URI))
84+
.scopes(Collections.singleton("default-scope"))
85+
.build());
86+
87+
completableFuture.get();
88+
Assert.assertEquals(capturedHttpRequest.getValues().size(),1);
89+
90+
}
91+
92+
/**
93+
* when instance_discovery flag is set to true (by default), an instance_discovery is NOT performed for b2c.
94+
*/
95+
@Test( dataProvider = "b2cAdfsClouds")
96+
public void b2cAdfsInstanceDiscoveryTrue(String authority) throws Exception{
97+
app = PowerMock.createPartialMock(PublicClientApplication.class,
98+
new String[]{"acquireTokenCommon"},
99+
PublicClientApplication.builder(TestConstants.ADFS_APP_ID)
100+
.authority(authority)
101+
.instanceDiscovery(true));
102+
103+
Capture<MsalRequest> capturedMsalRequest = Capture.newInstance();
104+
105+
PowerMock.expectPrivate(app, "acquireTokenCommon",
106+
EasyMock.capture(capturedMsalRequest), EasyMock.isA(AADAuthority.class)).andReturn(
107+
AuthenticationResult.builder().
108+
accessToken("accessToken").
109+
expiresOn(new Date().getTime() + 100).
110+
refreshToken("refreshToken").
111+
idToken("idToken").environment("environment").build());
112+
113+
PowerMock.mockStatic(HttpHelper.class);
114+
115+
HttpResponse instanceDiscoveryResponse = new HttpResponse();
116+
instanceDiscoveryResponse.statusCode(200);
117+
instanceDiscoveryResponse.body(TestConfiguration.INSTANCE_DISCOVERY_RESPONSE);
118+
119+
Capture<HttpRequest> capturedHttpRequest = Capture.newInstance();
120+
121+
EasyMock.expect(
122+
HttpHelper.executeHttpRequest(
123+
EasyMock.capture(capturedHttpRequest),
124+
EasyMock.isA(RequestContext.class),
125+
EasyMock.isA(ServiceBundle.class)))
126+
.andReturn(instanceDiscoveryResponse);
127+
128+
PowerMock.replay(HttpHelper.class, HttpResponse.class);
129+
130+
CompletableFuture<IAuthenticationResult> completableFuture = app.acquireToken(
131+
AuthorizationCodeParameters.builder
132+
("auth_code",
133+
new URI(TestConfiguration.AAD_DEFAULT_REDIRECT_URI))
134+
.scopes(Collections.singleton("default-scope"))
135+
.build());
136+
137+
completableFuture.get();
138+
Assert.assertEquals(capturedHttpRequest.getValues().size(),0);
139+
140+
}
141+
142+
/**
143+
* when instance_discovery flag is set to false, instance_discovery is not performed
144+
*/
145+
@Test (dataProvider = "aadClouds")
146+
public void aadInstanceDiscoveryFalse(String authority) throws Exception {
147+
148+
app = PowerMock.createPartialMock(PublicClientApplication.class,
149+
new String[]{"acquireTokenCommon"},
150+
PublicClientApplication.builder(TestConfiguration.AAD_CLIENT_ID)
151+
.authority(authority)
152+
.instanceDiscovery(false));
153+
154+
Capture<MsalRequest> capturedMsalRequest = Capture.newInstance();
155+
156+
PowerMock.expectPrivate(app, "acquireTokenCommon",
157+
EasyMock.capture(capturedMsalRequest), EasyMock.isA(AADAuthority.class)).andReturn(
158+
AuthenticationResult.builder().
159+
accessToken("accessToken").
160+
expiresOn(new Date().getTime() + 100).
161+
refreshToken("refreshToken").
162+
idToken("idToken").environment("environment").build());
163+
164+
PowerMock.mockStatic(HttpHelper.class);
165+
166+
HttpResponse instanceDiscoveryResponse = new HttpResponse();
167+
instanceDiscoveryResponse.statusCode(200);
168+
instanceDiscoveryResponse.body(TestConfiguration.INSTANCE_DISCOVERY_RESPONSE);
169+
170+
Capture<HttpRequest> capturedHttpRequest = Capture.newInstance();
171+
172+
EasyMock.expect(
173+
HttpHelper.executeHttpRequest(
174+
EasyMock.capture(capturedHttpRequest),
175+
EasyMock.isA(RequestContext.class),
176+
EasyMock.isA(ServiceBundle.class)))
177+
.andReturn(instanceDiscoveryResponse);
178+
179+
PowerMock.replay(HttpHelper.class, HttpResponse.class);
180+
181+
CompletableFuture<IAuthenticationResult> completableFuture = app.acquireToken(
182+
AuthorizationCodeParameters.builder
183+
("auth_code",
184+
new URI(TestConfiguration.AAD_DEFAULT_REDIRECT_URI))
185+
.scopes(Collections.singleton("default-scope"))
186+
.build());
187+
188+
completableFuture.get();
189+
Assert.assertEquals(capturedHttpRequest.getValues().size(),0);
190+
}
191+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
6767

6868
//If region autodetection is enabled and a specific region not already set,
6969
// set the application's region to the discovered region so that future requests can skip the IMDS endpoint call
70-
if (msalRequest.application().azureRegion() == null && msalRequest.application().autoDetectRegion()
70+
if (null == msalRequest.application().azureRegion() && msalRequest.application().autoDetectRegion()
7171
&& null != detectedRegion) {
7272
msalRequest.application().azureRegion = detectedRegion;
7373
}
@@ -83,11 +83,11 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
8383
doInstanceDiscoveryAndCache(authorityUrl, validateAuthority, msalRequest, serviceBundle);
8484
} else {
8585
// instanceDiscovery flag is set to False. Do not perform instanceDiscovery.
86-
cache.putIfAbsent(host, InstanceDiscoveryMetadataEntry.builder().
86+
return InstanceDiscoveryMetadataEntry.builder().
8787
preferredCache(host).
8888
preferredNetwork(host).
8989
aliases(Collections.singleton(host)).
90-
build());
90+
build();
9191
}
9292
}
9393

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

Lines changed: 3 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@
44
package com.microsoft.aad.msal4j;
55

66
import org.powermock.api.easymock.PowerMock;
7+
import org.powermock.core.classloader.annotations.PowerMockIgnore;
78
import org.powermock.core.classloader.annotations.PrepareForTest;
89
import org.powermock.modules.testng.PowerMockTestCase;
910
import org.testng.Assert;
1011
import org.testng.annotations.BeforeMethod;
11-
import org.testng.annotations.DataProvider;
1212
import org.testng.annotations.Test;
1313

1414
import java.net.URI;
1515
import java.net.URL;
16-
import java.util.Collections;
1716

18-
@PrepareForTest(AadInstanceDiscoveryProvider.class)
17+
@PowerMockIgnore({"javax.net.ssl.*"})
18+
@PrepareForTest({AadInstanceDiscoveryProvider.class})
1919
public class AadInstanceDiscoveryTest extends PowerMockTestCase {
2020

2121
@BeforeMethod
@@ -189,136 +189,4 @@ public void aadInstanceDiscoveryTest_AutoDetectRegion_NoRegionDetected() throws
189189
Assert.assertTrue(entry.aliases().contains("sts.windows.net"));
190190
}
191191

192-
@DataProvider(name = "aadClouds")
193-
private static Object[][] getAadClouds(){
194-
return new Object[][] {{"https://login.microsoftonline.com/common"} , // #Known to Microsoft
195-
{"https://private.cloud/foo"}//Private Cloud
196-
};
197-
}
198-
199-
@DataProvider(name = "b2cAdfsClouds")
200-
private static Object[][] getNonAadClouds(){
201-
return new Object[][] {{"https://contoso.com/adfs"}//ADFS
202-
// {"https://login.b2clogin.com/contoso/b2c_policy"},//B2C
203-
};
204-
}
205-
206-
/**
207-
* when instance_discovery flag is set to true (by default), an instance_discovery is performed for authorityType = AAD and
208-
* hence, an exception is thrown while making a call to getMetaDataEntry() if instanceDiscoveryResponse is not mocked.
209-
*/
210-
@Test( dataProvider = "aadClouds",
211-
expectedExceptions = StringIndexOutOfBoundsException.class)
212-
public void aad_instance_discovery_true(String authority) throws Exception {
213-
214-
PublicClientApplication app = PublicClientApplication.builder("client_id")
215-
.authority(authority)
216-
.build();
217-
218-
AuthorizationCodeParameters parameters = AuthorizationCodeParameters.builder(
219-
"code", new URI("http://my.redirect.com"))
220-
.scopes(Collections.singleton("scope")).build();
221-
222-
MsalRequest msalRequest = new AuthorizationCodeRequest(
223-
parameters,
224-
app,
225-
new RequestContext(app, PublicApi.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE, parameters));
226-
227-
URL authorityURL = new URL(authority);
228-
229-
AadInstanceDiscoveryProvider.getMetadataEntry(
230-
authorityURL,
231-
false,
232-
msalRequest,
233-
app.getServiceBundle());
234-
235-
}
236-
237-
/**
238-
* when instance_discovery flag is set to true (by default), an instance_discovery is NOT performed for b2c.
239-
*/
240-
@Test( dataProvider = "b2cAdfsClouds")
241-
public void b2c_adfs_instance_discovery_true(String authority) throws Exception {
242-
243-
PublicClientApplication app = PublicClientApplication.builder("client_id")
244-
.authority(authority)
245-
.build();
246-
247-
AuthorizationCodeParameters parameters = AuthorizationCodeParameters.builder(
248-
"code", new URI("http://my.redirect.com"))
249-
.scopes(Collections.singleton("scope")).build();
250-
251-
MsalRequest msalRequest = new AuthorizationCodeRequest(
252-
parameters,
253-
app,
254-
new RequestContext(app, PublicApi.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE, parameters));
255-
256-
URL authorityURL = new URL(authority);
257-
258-
AadInstanceDiscoveryProvider.getMetadataEntry(
259-
authorityURL,
260-
false,
261-
msalRequest,
262-
app.getServiceBundle());
263-
}
264-
265-
@Test (dataProvider = "aadClouds")
266-
/**
267-
* when instance_discovery flag is set to false, instance_discovery is not performed and hence,
268-
* no exception is thrown while making a call to getMetaDataEntry() even when instanceDiscoveryResponse is not mocked.
269-
*/
270-
public void aad_instance_discovery_false(String authority) throws Exception{
271-
272-
PublicClientApplication app = PublicClientApplication.builder("client_id")
273-
.authority(authority)
274-
.instanceDiscovery(false)
275-
.build();
276-
277-
AuthorizationCodeParameters parameters = AuthorizationCodeParameters.builder(
278-
"code", new URI("http://my.redirect.com"))
279-
.scopes(Collections.singleton("scope")).build();
280-
281-
MsalRequest msalRequest = new AuthorizationCodeRequest(
282-
parameters,
283-
app,
284-
new RequestContext(app, PublicApi.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE, parameters));
285-
286-
URL authorityURL = new URL(authority);
287-
288-
AadInstanceDiscoveryProvider.getMetadataEntry(
289-
authorityURL,
290-
false,
291-
msalRequest,
292-
app.getServiceBundle());
293-
}
294-
295-
@Test (dataProvider = "b2cAdfsClouds")
296-
/**
297-
* when instance_discovery flag is set to true, instance_discovery is not performed and hence,
298-
* no exception is thrown while making a call to getMetaDataEntry() even when instanceDiscoveryResponse is not mocked.
299-
*/
300-
public void b2c_adfs_instance_discovery_false(String authority) throws Exception{
301-
302-
PublicClientApplication app = PublicClientApplication.builder("client_id")
303-
.authority(authority)
304-
.instanceDiscovery(false)
305-
.build();
306-
307-
AuthorizationCodeParameters parameters = AuthorizationCodeParameters.builder(
308-
"code", new URI("http://my.redirect.com"))
309-
.scopes(Collections.singleton("scope")).build();
310-
311-
MsalRequest msalRequest = new AuthorizationCodeRequest(
312-
parameters,
313-
app,
314-
new RequestContext(app, PublicApi.ACQUIRE_TOKEN_BY_AUTHORIZATION_CODE, parameters));
315-
316-
URL authorityURL = new URL(authority);
317-
318-
AadInstanceDiscoveryProvider.getMetadataEntry(
319-
authorityURL,
320-
false,
321-
msalRequest,
322-
app.getServiceBundle());
323-
}
324192
}

0 commit comments

Comments
 (0)