Skip to content

Commit 6947556

Browse files
Avery-DunnAvery-Dunn
committed
PR feedback around tests
1 parent f1fa204 commit 6947556

1 file changed

Lines changed: 83 additions & 45 deletions

File tree

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

Lines changed: 83 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -393,13 +393,13 @@ private HttpResponse createUserResponse(String oid, String preferredUsername, St
393393
@Test
394394
void userFic_TwoUpns_SilentReturnsCorrectToken() throws Exception {
395395
// Arrange
396-
String alice_oid = "oid-alice-1111";
397-
String alice_upn = "alice@contoso.com";
398-
String alice_token = "access-token-alice";
396+
String aliceOid = "oid-alice-1111";
397+
String aliceUpn = "alice@contoso.com";
398+
String aliceToken = "access-token-alice";
399399

400-
String bob_oid = "oid-bob-2222";
401-
String bob_upn = "bob@contoso.com";
402-
String bob_token = "access-token-bob";
400+
String bobOid = "oid-bob-2222";
401+
String bobUpn = "bob@contoso.com";
402+
String bobToken = "access-token-bob";
403403

404404
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
405405

@@ -408,23 +408,23 @@ void userFic_TwoUpns_SilentReturnsCorrectToken() throws Exception {
408408
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> nextResponse.get());
409409

410410
// Act: Acquire token for Alice
411-
nextResponse.set(createUserResponse(alice_oid, alice_upn, alice_token, tid));
411+
nextResponse.set(createUserResponse(aliceOid, aliceUpn, aliceToken, tid));
412412

413413
UserFederatedIdentityCredentialParameters aliceParams = UserFederatedIdentityCredentialParameters
414-
.builder(SCOPES, alice_upn, FAKE_ASSERTION)
414+
.builder(SCOPES, aliceUpn, FAKE_ASSERTION)
415415
.build();
416416
IAuthenticationResult aliceResult = cca.acquireToken(aliceParams).get();
417-
assertEquals(alice_token, aliceResult.accessToken());
417+
assertEquals(aliceToken, aliceResult.accessToken());
418418
assertNotNull(aliceResult.account());
419419

420420
// Act: Acquire token for Bob
421-
nextResponse.set(createUserResponse(bob_oid, bob_upn, bob_token, tid));
421+
nextResponse.set(createUserResponse(bobOid, bobUpn, bobToken, tid));
422422

423423
UserFederatedIdentityCredentialParameters bobParams = UserFederatedIdentityCredentialParameters
424-
.builder(SCOPES, bob_upn, FAKE_ASSERTION)
424+
.builder(SCOPES, bobUpn, FAKE_ASSERTION)
425425
.build();
426426
IAuthenticationResult bobResult = cca.acquireToken(bobParams).get();
427-
assertEquals(bob_token, bobResult.accessToken());
427+
assertEquals(bobToken, bobResult.accessToken());
428428
assertNotNull(bobResult.account());
429429

430430
// Assert: Both accounts in cache
@@ -433,21 +433,21 @@ void userFic_TwoUpns_SilentReturnsCorrectToken() throws Exception {
433433

434434
// Silent for Alice → should return Alice's token
435435
IAccount aliceAccount = accounts.stream()
436-
.filter(a -> alice_upn.equalsIgnoreCase(a.username()))
436+
.filter(a -> aliceUpn.equalsIgnoreCase(a.username()))
437437
.findFirst().orElse(null);
438438
assertNotNull(aliceAccount, "Alice account should be in cache");
439439
IAuthenticationResult silentAlice = cca.acquireTokenSilently(
440440
SilentParameters.builder(SCOPES, aliceAccount).build()).get();
441-
assertEquals(alice_token, silentAlice.accessToken(), "Silent for Alice should return Alice's token");
441+
assertEquals(aliceToken, silentAlice.accessToken(), "Silent for Alice should return Alice's token");
442442

443443
// Silent for Bob → should return Bob's token
444444
IAccount bobAccount = accounts.stream()
445-
.filter(a -> bob_upn.equalsIgnoreCase(a.username()))
445+
.filter(a -> bobUpn.equalsIgnoreCase(a.username()))
446446
.findFirst().orElse(null);
447447
assertNotNull(bobAccount, "Bob account should be in cache");
448448
IAuthenticationResult silentBob = cca.acquireTokenSilently(
449449
SilentParameters.builder(SCOPES, bobAccount).build()).get();
450-
assertEquals(bob_token, silentBob.accessToken(), "Silent for Bob should return Bob's token");
450+
assertEquals(bobToken, silentBob.accessToken(), "Silent for Bob should return Bob's token");
451451
}
452452

453453
/**
@@ -457,13 +457,13 @@ void userFic_TwoUpns_SilentReturnsCorrectToken() throws Exception {
457457
@Test
458458
void userFic_TwoOids_SilentReturnsCorrectToken() throws Exception {
459459
// Arrange
460-
String carol_oid = "oid-carol-3333";
461-
String carol_upn = "carol@contoso.com";
462-
String carol_token = "access-token-carol";
460+
String carolOid = "oid-carol-3333";
461+
String carolUpn = "carol@contoso.com";
462+
String carolToken = "access-token-carol";
463463

464-
String dave_oid = "oid-dave-4444";
465-
String dave_upn = "dave@contoso.com";
466-
String dave_token = "access-token-dave";
464+
String daveOid = "oid-dave-4444";
465+
String daveUpn = "dave@contoso.com";
466+
String daveToken = "access-token-dave";
467467

468468
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
469469

@@ -472,44 +472,82 @@ void userFic_TwoOids_SilentReturnsCorrectToken() throws Exception {
472472
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> nextResponse.get());
473473

474474
// Act: Acquire token for Carol (using UPN)
475-
nextResponse.set(createUserResponse(carol_oid, carol_upn, carol_token, tid));
475+
nextResponse.set(createUserResponse(carolOid, carolUpn, carolToken, tid));
476476

477477
UserFederatedIdentityCredentialParameters carolParams = UserFederatedIdentityCredentialParameters
478-
.builder(SCOPES, carol_upn, FAKE_ASSERTION)
478+
.builder(SCOPES, carolUpn, FAKE_ASSERTION)
479479
.build();
480480
IAuthenticationResult carolResult = cca.acquireToken(carolParams).get();
481-
assertEquals(carol_token, carolResult.accessToken());
481+
assertEquals(carolToken, carolResult.accessToken());
482482

483483
// Act: Acquire token for Dave (using UPN)
484-
nextResponse.set(createUserResponse(dave_oid, dave_upn, dave_token, tid));
484+
nextResponse.set(createUserResponse(daveOid, daveUpn, daveToken, tid));
485485

486486
UserFederatedIdentityCredentialParameters daveParams = UserFederatedIdentityCredentialParameters
487-
.builder(SCOPES, dave_upn, FAKE_ASSERTION)
487+
.builder(SCOPES, daveUpn, FAKE_ASSERTION)
488488
.build();
489489
IAuthenticationResult daveResult = cca.acquireToken(daveParams).get();
490-
assertEquals(dave_token, daveResult.accessToken());
490+
assertEquals(daveToken, daveResult.accessToken());
491491

492492
// Assert: Both accounts in cache
493493
Set<IAccount> accounts = cca.getAccounts().get();
494494
assertEquals(2, accounts.size(), "Two accounts should be cached");
495495

496496
// Lookup by OID for Carol
497497
IAccount carolAccount = accounts.stream()
498-
.filter(a -> a.homeAccountId().contains(carol_oid))
498+
.filter(a -> a.homeAccountId().contains(carolOid))
499499
.findFirst().orElse(null);
500500
assertNotNull(carolAccount, "Carol account should be in cache");
501501
IAuthenticationResult silentCarol = cca.acquireTokenSilently(
502502
SilentParameters.builder(SCOPES, carolAccount).build()).get();
503-
assertEquals(carol_token, silentCarol.accessToken(), "OID-based lookup for Carol should return Carol's token");
503+
assertEquals(carolToken, silentCarol.accessToken(), "OID-based lookup for Carol should return Carol's token");
504504

505505
// Lookup by OID for Dave
506506
IAccount daveAccount = accounts.stream()
507-
.filter(a -> a.homeAccountId().contains(dave_oid))
507+
.filter(a -> a.homeAccountId().contains(daveOid))
508508
.findFirst().orElse(null);
509509
assertNotNull(daveAccount, "Dave account should be in cache");
510510
IAuthenticationResult silentDave = cca.acquireTokenSilently(
511511
SilentParameters.builder(SCOPES, daveAccount).build()).get();
512-
assertEquals(dave_token, silentDave.accessToken(), "OID-based lookup for Dave should return Dave's token");
512+
assertEquals(daveToken, silentDave.accessToken(), "OID-based lookup for Dave should return Dave's token");
513+
}
514+
515+
/**
516+
* Verifies that the UUID builder overload combined with findCachedAccount()'s OID-based
517+
* matching produces a cache hit on the second call (no second network request).
518+
*/
519+
@Test
520+
void userFic_UuidOverload_CacheHitOnSecondCall() throws Exception {
521+
// Arrange
522+
UUID userOid = UUID.fromString("11111111-2222-3333-4444-555555555555");
523+
String oidStr = userOid.toString();
524+
String upn = "eve@contoso.com";
525+
String token = "access-token-eve";
526+
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
527+
528+
AtomicInteger networkCalls = new AtomicInteger(0);
529+
530+
when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> {
531+
networkCalls.incrementAndGet();
532+
return createUserResponse(oidStr, upn, token, tid);
533+
});
534+
535+
// Act 1: First call — goes to IdP
536+
UserFederatedIdentityCredentialParameters params = UserFederatedIdentityCredentialParameters
537+
.builder(SCOPES, userOid, FAKE_ASSERTION)
538+
.build();
539+
IAuthenticationResult result1 = cca.acquireToken(params).get();
540+
assertEquals(token, result1.accessToken());
541+
assertEquals(1, networkCalls.get(), "First call should hit the network");
542+
543+
// Act 2: Second call with same UUID and forceRefresh=false — should use cache
544+
UserFederatedIdentityCredentialParameters params2 = UserFederatedIdentityCredentialParameters
545+
.builder(SCOPES, userOid, FAKE_ASSERTION)
546+
.forceRefresh(false)
547+
.build();
548+
IAuthenticationResult result2 = cca.acquireToken(params2).get();
549+
assertEquals(token, result2.accessToken());
550+
assertEquals(1, networkCalls.get(), "Second call should NOT hit the network (cache hit via OID match)");
513551
}
514552

515553
/**
@@ -523,13 +561,13 @@ void userFic_TwoOids_SilentReturnsCorrectToken() throws Exception {
523561
@Test
524562
void userFic_SingleAccountFallback_DoesNotReturnWrongUserToken() throws Exception {
525563
// Arrange
526-
String alice_oid = "oid-alice-1111";
527-
String alice_upn = "alice@contoso.com";
528-
String alice_token = "access-token-alice";
564+
String aliceOid = "oid-alice-1111";
565+
String aliceUpn = "alice@contoso.com";
566+
String aliceToken = "access-token-alice";
529567

530-
String bob_oid = "oid-bob-2222";
531-
String bob_upn = "bob@contoso.com";
532-
String bob_token = "access-token-bob";
568+
String bobOid = "oid-bob-2222";
569+
String bobUpn = "bob@contoso.com";
570+
String bobToken = "access-token-bob";
533571

534572
String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
535573

@@ -542,13 +580,13 @@ void userFic_SingleAccountFallback_DoesNotReturnWrongUserToken() throws Exceptio
542580
});
543581

544582
// Act 1: Acquire token for Alice (goes to IdP, gets cached)
545-
nextResponse.set(createUserResponse(alice_oid, alice_upn, alice_token, tid));
583+
nextResponse.set(createUserResponse(aliceOid, aliceUpn, aliceToken, tid));
546584

547585
UserFederatedIdentityCredentialParameters aliceParams = UserFederatedIdentityCredentialParameters
548-
.builder(SCOPES, alice_upn, FAKE_ASSERTION)
586+
.builder(SCOPES, aliceUpn, FAKE_ASSERTION)
549587
.build();
550588
IAuthenticationResult aliceResult = cca.acquireToken(aliceParams).get();
551-
assertEquals(alice_token, aliceResult.accessToken());
589+
assertEquals(aliceToken, aliceResult.accessToken());
552590
int callsAfterAlice = networkCalls.get();
553591

554592
// At this point, cache has exactly one account (Alice).
@@ -558,17 +596,17 @@ void userFic_SingleAccountFallback_DoesNotReturnWrongUserToken() throws Exceptio
558596
// findCachedAccount() runs; there is one account (Alice) but it doesn't match Bob.
559597
// The correct behavior is to return null → go to IdP → get Bob's token.
560598
// The buggy behavior was: accounts.size()==1 → return Alice → return Alice's cached token.
561-
nextResponse.set(createUserResponse(bob_oid, bob_upn, bob_token, tid));
599+
nextResponse.set(createUserResponse(bobOid, bobUpn, bobToken, tid));
562600

563601
UserFederatedIdentityCredentialParameters bobParams = UserFederatedIdentityCredentialParameters
564-
.builder(SCOPES, bob_upn, FAKE_ASSERTION)
602+
.builder(SCOPES, bobUpn, FAKE_ASSERTION)
565603
.build();
566604
IAuthenticationResult bobResult = cca.acquireToken(bobParams).get();
567605

568606
// Assert: Bob should have gotten his own token, not Alice's
569-
assertEquals(bob_token, bobResult.accessToken(),
607+
assertEquals(bobToken, bobResult.accessToken(),
570608
"Bob should receive his own token, not Alice's cached token");
571-
assertNotEquals(alice_token, bobResult.accessToken(),
609+
assertNotEquals(aliceToken, bobResult.accessToken(),
572610
"Bob's token must differ from Alice's");
573611

574612
// Assert: A network call was made for Bob (not just a cache hit)

0 commit comments

Comments
 (0)