PR feedback, refactor common code

Author: Avery-Dunn

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

@@ -20,28 +20,11 @@
 @ExtendWith(MockitoExtension.class)
 class OnBehalfOfTests {
 
-    private String getSuccessfulResponse(String accessToken) {
-        return "{\"access_token\":\""+accessToken+"\",\"expires_in\": \""+ 60*60*1000 +"\",\"token_type\":" +
-                "\"Bearer\",\"client_id\":\"client_id\",\"Content-Type\":\"text/html; charset=utf-8\"}";
-    }
-
-    private HttpResponse expectedResponse(int statusCode, String response) {
-        Map<String, List<String>> headers = new HashMap<String, List<String>>();
-        headers.put("Content-Type", Collections.singletonList("application/json"));
-
-        HttpResponse httpResponse = new HttpResponse();
-        httpResponse.statusCode(statusCode);
-        httpResponse.body(response);
-        httpResponse.addHeaders(headers);
-
-        return httpResponse;
-    }
-
     @Test
     void OnBehalfOf_InternalCacheLookup_Success() throws Exception {
         DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class);
 
-        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(expectedResponse(200, getSuccessfulResponse("token")));
+        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(TestHelper.expectedResponse(200, TestHelper.getSuccessfulTokenResponse(new HashMap<>())));
 
         ConfidentialClientApplication cca =
                 ConfidentialClientApplication.builder("clientId", ClientCredentialFactory.createFromSecret("password"))
@@ -51,7 +34,7 @@ void OnBehalfOf_InternalCacheLookup_Success() throws Exception {
                         .httpClient(httpClientMock)
                         .build();
 
-        OnBehalfOfParameters parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedToken)).build();
+        OnBehalfOfParameters parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedAssertion)).build();
 
         IAuthenticationResult result = cca.acquireToken(parameters).get();
         IAuthenticationResult result2 = cca.acquireToken(parameters).get();
@@ -73,23 +56,32 @@ void OnBehalfOf_TenantOverride() throws Exception {
                         .httpClient(httpClientMock)
                         .build();
 
-        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(expectedResponse(200, getSuccessfulResponse("appTenantToken")));
-        OnBehalfOfParameters parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedToken)).build();
+        HashMap<String, String> tokenResponseValues = new HashMap<>();
+        tokenResponseValues.put("access_token", "accessTokenFirstCall");
+
+        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(TestHelper.expectedResponse(200, TestHelper.getSuccessfulTokenResponse(tokenResponseValues)));
+        OnBehalfOfParameters parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedAssertion)).build();
 
-        //The two acquireToken calls have the same parameters and should only cause one call from the HTTP client
+        //The two acquireToken calls have the same parameters...
         IAuthenticationResult resultAppLevelTenant = cca.acquireToken(parameters).get();
-        cca.acquireToken(parameters).get();
+        IAuthenticationResult resultAppLevelTenantCached = cca.acquireToken(parameters).get();
+        //...so only one token should be added to the cache, and the mocked HTTP client's "send" method should only have been called once
         assertEquals(1, cca.tokenCache.accessTokens.size());
+        assertEquals(resultAppLevelTenant.accessToken(), resultAppLevelTenantCached.accessToken());
         verify(httpClientMock, times(1)).send(any());
 
-        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(expectedResponse(200, getSuccessfulResponse("requestTenantToken")));
-        parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedToken)).tenant("otherTenant").build();
+        tokenResponseValues.put("access_token", "accessTokenSecondCall");
+
+        when(httpClientMock.send(any(HttpRequest.class))).thenReturn(TestHelper.expectedResponse(200, TestHelper.getSuccessfulTokenResponse(tokenResponseValues)));
+        parameters = OnBehalfOfParameters.builder(Collections.singleton("scopes"), new UserAssertion(TestHelper.signedAssertion)).tenant("otherTenant").build();
 
-        //Overriding the tenant parameter in the request should lead to a new token call being made, but followup calls should not
+        //Overriding the tenant parameter in the request should lead to a new token call being made...
         IAuthenticationResult resultRequestLevelTenant = cca.acquireToken(parameters).get();
-        cca.acquireToken(parameters).get();
+        IAuthenticationResult resultRequestLevelTenantCached = cca.acquireToken(parameters).get();
+        //...which should be different from the original token, and thus the cache should have two tokens created from two HTTP calls
         assertEquals(2, cca.tokenCache.accessTokens.size());
-        verify(httpClientMock, times(2)).send(any());
+        assertEquals(resultRequestLevelTenant.accessToken(), resultRequestLevelTenantCached.accessToken());
         assertNotEquals(resultAppLevelTenant.accessToken(), resultRequestLevelTenant.accessToken());
+        verify(httpClientMock, times(2)).send(any());
     }
 }

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

@@ -14,12 +14,20 @@
 import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 class TestHelper {
 
     //Signed JWT which should be enough to pass the parsing/validation in the library, useful if a unit test needs an
-    // assertion in a request or token in a response but that is not the focus of the test
-    static String signedToken = generateToken();
+    // assertion but that is not the focus of the test
+    static String signedAssertion = generateToken();
+    private static final String successfulResponseFormat = "{\"access_token\":\"%s\",\"id_token\":\"%s\",\"refresh_token\":\"%s\"," +
+            "\"client_id\":\"%s\",\"client_info\":\"%s\"," +
+            "\"expires_on\": %d ,\"expires_in\": %d," +
+            "\"token_type\":\"Bearer\"}";
 
     static String readResource(Class<?> classInstance, String resource) {
         try {
@@ -55,4 +63,38 @@ static String generateToken() {
             throw new RuntimeException(e);
         }
     }
+
+    //Maps various values to the successfulResponseFormat string to create a valid token response
+    static String getSuccessfulTokenResponse(HashMap<String, String> responseValues) {
+        //Will default to expiring in one hour if expiry time values are not set
+        long expiresIn = responseValues.containsKey("expires_in") ?
+                Long.parseLong(responseValues.get("expires_in")) :
+                3600;
+        long expiresOn = responseValues.containsKey("expires_on")
+                ? Long.parseLong(responseValues.get("expires_0n")) :
+                (System.currentTimeMillis() / 1000) + expiresIn;
+
+        return String.format(successfulResponseFormat,
+                responseValues.getOrDefault("access_token", "access_token"),
+                responseValues.getOrDefault("id_token", "id_token"),
+                responseValues.getOrDefault("refresh_token", "refresh_token"),
+                responseValues.getOrDefault("client_id", "client_id"),
+                responseValues.getOrDefault("client_info", "client_info"),
+                expiresOn,
+                expiresIn
+        );
+    }
+
+    //Creates a valid HttpResponse that can be used when mocking HttpClient.send()
+    static HttpResponse expectedResponse(int statusCode, String response) {
+        Map<String, List<String>> headers = new HashMap<>();
+        headers.put("Content-Type", Collections.singletonList("application/json"));
+
+        HttpResponse httpResponse = new HttpResponse();
+        httpResponse.statusCode(statusCode);
+        httpResponse.body(response);
+        httpResponse.addHeaders(headers);
+
+        return httpResponse;
+    }
 }