Skip to content

Commit 09ea0f5

Browse files
committed
PR feedback
1 parent 61a6c88 commit 09ea0f5

3 files changed

Lines changed: 15 additions & 52 deletions

File tree

msal4j-sdk/src/integrationtest/java/com/microsoft/aad/msal4j/AgenticIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,8 @@ void agentCca_AppAndUserTokens_CacheIsolation() throws Exception {
294294
"App token and user token should be different");
295295

296296
// App cache should have 1 entry, user cache should have user account
297-
assertTrue(agentCca.tokenCache.accessTokens.size() >= 2,
298-
"Cache should have at least 2 entries (app + user)");
297+
assertEquals(2, agentCca.tokenCache.accessTokens.size(),
298+
"Cache should have exactly 2 entries (app + user)");
299299
}
300300

301301
/**

msal4j-sdk/src/integrationtest/java/com/microsoft/aad/msal4j/FicIT.java renamed to msal4j-sdk/src/integrationtest/java/com/microsoft/aad/msal4j/UserFicIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
* </ul>
4343
*/
4444
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
45-
class FicIT {
45+
class UserFicIT {
4646

4747
// Same config as AgenticIT
4848
private static final String BLUEPRINT_CLIENT_ID = "aab5089d-e764-47e3-9f28-cc11c2513821";

msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialTest.java

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package com.microsoft.aad.msal4j;
55

6+
import org.junit.jupiter.api.BeforeEach;
67
import org.junit.jupiter.api.Test;
78
import org.junit.jupiter.api.TestInstance;
89

@@ -32,6 +33,16 @@ class UserFederatedIdentityCredentialTest {
3233
private static final String TEST_UPN = "user@contoso.com";
3334
private static final UUID TEST_OID = UUID.fromString("597f86cd-13f3-44c0-bece-a1e77ba43228");
3435

36+
private DefaultHttpClient httpClientMock;
37+
private ConfidentialClientApplication cca;
38+
39+
@BeforeEach
40+
void setUp() throws Exception {
41+
httpClientMock = mock(DefaultHttpClient.class);
42+
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
43+
cca = createCca(httpClientMock);
44+
}
45+
3546
private ConfidentialClientApplication createCca(DefaultHttpClient httpClientMock) throws Exception {
3647
return ConfidentialClientApplication.builder(CLIENT_ID, ClientCredentialFactory.createFromSecret("secret"))
3748
.authority(AUTHORITY)
@@ -63,11 +74,6 @@ private HttpResponse createSuccessResponseWithIdToken() {
6374
@Test
6475
void userFic_SendsCorrectGrantType() throws Exception {
6576
// Arrange
66-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
67-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
68-
69-
ConfidentialClientApplication cca = createCca(httpClientMock);
70-
7177
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
7278
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
7379
.build();
@@ -89,11 +95,6 @@ void userFic_SendsCorrectGrantType() throws Exception {
8995
@Test
9096
void userFic_SendsAssertionInBody() throws Exception {
9197
// Arrange
92-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
93-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
94-
95-
ConfidentialClientApplication cca = createCca(httpClientMock);
96-
9798
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
9899
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
99100
.build();
@@ -115,11 +116,6 @@ void userFic_SendsAssertionInBody() throws Exception {
115116
@Test
116117
void userFic_WithUpn_SendsUsernameNotUserId() throws Exception {
117118
// Arrange
118-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
119-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
120-
121-
ConfidentialClientApplication cca = createCca(httpClientMock);
122-
123119
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
124120
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
125121
.build();
@@ -138,11 +134,6 @@ void userFic_WithUpn_SendsUsernameNotUserId() throws Exception {
138134
@Test
139135
void userFic_WithOid_SendsUserIdNotUsername() throws Exception {
140136
// Arrange
141-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
142-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
143-
144-
ConfidentialClientApplication cca = createCca(httpClientMock);
145-
146137
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
147138
.builder(SCOPES, TEST_OID, FAKE_ASSERTION)
148139
.build();
@@ -165,11 +156,6 @@ void userFic_WithOid_SendsUserIdNotUsername() throws Exception {
165156
@Test
166157
void userFic_SendsAllOAuthParametersTogether() throws Exception {
167158
// Arrange
168-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
169-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
170-
171-
ConfidentialClientApplication cca = createCca(httpClientMock);
172-
173159
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
174160
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
175161
.build();
@@ -194,11 +180,6 @@ void userFic_SendsAllOAuthParametersTogether() throws Exception {
194180
@Test
195181
void userFic_ScopeIncludesOidcScopes() throws Exception {
196182
// Arrange
197-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
198-
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponse());
199-
200-
ConfidentialClientApplication cca = createCca(httpClientMock);
201-
202183
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
203184
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
204185
.build();
@@ -221,12 +202,9 @@ void userFic_ScopeIncludesOidcScopes() throws Exception {
221202

222203
@Test
223204
void userFic_TokenStoredInUserCache() throws Exception {
224-
// Arrange
225-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
205+
// Arrange — override default mock to return id_token
226206
when(httpClientMock.send(any(HttpRequest.class))).thenReturn(createSuccessResponseWithIdToken());
227207

228-
ConfidentialClientApplication cca = createCca(httpClientMock);
229-
230208
UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters
231209
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
232210
.build();
@@ -252,14 +230,11 @@ void userFic_ForceRefresh_BypassesCache() throws Exception {
252230
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
253231
AtomicInteger callCount = new AtomicInteger(0);
254232

255-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
256233
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> {
257234
callCount.incrementAndGet();
258235
return createUserResponse(oid, TEST_UPN, "access-token", tid);
259236
});
260237

261-
ConfidentialClientApplication cca = createCca(httpClientMock);
262-
263238
// First call — populates cache (empty cache, goes to IdP)
264239
UserFederatedIdentityCredentialParameters params1 = UserFederatedIdentityCredentialParameters
265240
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
@@ -287,14 +262,11 @@ void userFic_CacheHit_WhenNotForceRefreshing() throws Exception {
287262
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
288263
AtomicInteger callCount = new AtomicInteger(0);
289264

290-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
291265
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> {
292266
callCount.incrementAndGet();
293267
return createUserResponse(oid, TEST_UPN, "access-token-cached", tid);
294268
});
295269

296-
ConfidentialClientApplication cca = createCca(httpClientMock);
297-
298270
// First call — populates cache (cache is empty, so goes to IdP without needing forceRefresh)
299271
UserFederatedIdentityCredentialParameters params1 = UserFederatedIdentityCredentialParameters
300272
.builder(SCOPES, TEST_UPN, FAKE_ASSERTION)
@@ -433,11 +405,8 @@ void userFic_TwoUpns_SilentReturnsCorrectToken() throws Exception {
433405

434406
AtomicReference<HttpResponse> nextResponse = new AtomicReference<>();
435407

436-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
437408
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> nextResponse.get());
438409

439-
ConfidentialClientApplication cca = createCca(httpClientMock);
440-
441410
// Act: Acquire token for Alice
442411
nextResponse.set(createUserResponse(alice_oid, alice_upn, alice_token, tid));
443412

@@ -500,11 +469,8 @@ void userFic_TwoOids_SilentReturnsCorrectToken() throws Exception {
500469

501470
AtomicReference<HttpResponse> nextResponse = new AtomicReference<>();
502471

503-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
504472
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> nextResponse.get());
505473

506-
ConfidentialClientApplication cca = createCca(httpClientMock);
507-
508474
// Act: Acquire token for Carol (using UPN)
509475
nextResponse.set(createUserResponse(carol_oid, carol_upn, carol_token, tid));
510476

@@ -570,14 +536,11 @@ void userFic_SingleAccountFallback_DoesNotReturnWrongUserToken() throws Exceptio
570536
AtomicInteger networkCalls = new AtomicInteger(0);
571537
AtomicReference<HttpResponse> nextResponse = new AtomicReference<>();
572538

573-
DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
574539
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> {
575540
networkCalls.incrementAndGet();
576541
return nextResponse.get();
577542
});
578543

579-
ConfidentialClientApplication cca = createCca(httpClientMock);
580-
581544
// Act 1: Acquire token for Alice (goes to IdP, gets cached)
582545
nextResponse.set(createUserResponse(alice_oid, alice_upn, alice_token, tid));
583546

0 commit comments

Comments
 (0)