From 9e13c38b4b14d3aa9a6873cf64981ab7755a48e0 Mon Sep 17 00:00:00 2001 From: avdunn Date: Tue, 7 Jan 2025 14:22:40 -0800 Subject: [PATCH 1/2] Add jacoco config and update surefire plugin --- msal4j-sdk/pom.xml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/msal4j-sdk/pom.xml b/msal4j-sdk/pom.xml index 9dd0d24e..ea0f05c1 100644 --- a/msal4j-sdk/pom.xml +++ b/msal4j-sdk/pom.xml @@ -239,9 +239,9 @@ org.apache.maven.plugins maven-surefire-plugin - 2.10 + 3.5.2 - -noverify + @{argLine} -noverify @@ -332,6 +332,25 @@ + + org.jacoco + jacoco-maven-plugin + 0.8.12 + + + + prepare-agent + + + + jacoco-site + test + + report + + + + From d58d5a8c00dcf673b0060ed259284c6307f95cc2 Mon Sep 17 00:00:00 2001 From: avdunn Date: Mon, 27 Jan 2025 15:47:44 -0800 Subject: [PATCH 2/2] Fix MI unit tests --- .../ServiceFabricManagedIdentitySource.java | 10 ++++- .../aad/msal4j/ManagedIdentityTests.java | 41 +++++++++++++++---- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java index 6c20cf9c..5368eff2 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java @@ -24,8 +24,8 @@ class ServiceFabricManagedIdentitySource extends AbstractManagedIdentitySource { //Service Fabric requires a special check for an environment variable containing a certificate thumbprint used for validating requests. //No other flow need this and an app developer may not be aware of it, so it was decided that for the Service Fabric flow we will simply override // any HttpClient that may have been set by the app developer with our own client which performs the validation logic. - private final IHttpClient httpClient = new DefaultHttpClientManagedIdentity(null, null, null, null); - private final HttpHelper httpHelper = new HttpHelper(httpClient); + private static IHttpClient httpClient = new DefaultHttpClientManagedIdentity(null, null, null, null); + private static HttpHelper httpHelper = new HttpHelperManagedIdentity(httpClient); @Override public void createManagedIdentityRequest(String resource) { @@ -117,4 +117,10 @@ private static URI validateAndGetUri(String msiEndpoint) } } + //The HttpClient is not normally customizable in this flow, as it requires special behavior for certificate validation. + //However, unit tests often need to mock HttpClient and need a way to inject the mocked object into this class. + static void setHttpClient(IHttpClient client) { + httpClient = client; + httpHelper = new HttpHelperManagedIdentity(httpClient); + } } \ No newline at end of file diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java index aac0cbf6..312f683a 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java @@ -71,7 +71,6 @@ private HttpRequest expectedRequest(ManagedIdentitySourceType source, String res String endpoint = null; Map headers = new HashMap<>(); Map> queryParameters = new HashMap<>(); - Map> bodyParameters = new HashMap<>(); switch (source) { case APP_SERVICE: { @@ -89,10 +88,8 @@ private HttpRequest expectedRequest(ManagedIdentitySourceType source, String res headers.put("ContentType", "application/x-www-form-urlencoded"); headers.put("Metadata", "true"); - bodyParameters.put("resource", singletonList(resource)); - queryParameters.put("resource", singletonList(resource)); - return new HttpRequest(HttpMethod.GET, computeUri(endpoint, queryParameters), headers, URLUtils.serializeParameters(bodyParameters)); + break; } case IMDS: { endpoint = IMDS_ENDPOINT; @@ -110,11 +107,14 @@ private HttpRequest expectedRequest(ManagedIdentitySourceType source, String res headers.put("Metadata", "true"); break; } - case SERVICE_FABRIC: + case SERVICE_FABRIC: { endpoint = serviceFabricEndpoint; queryParameters.put("api-version", singletonList("2019-07-01-preview")); queryParameters.put("resource", singletonList(resource)); + + headers.put("secret", "secret"); break; + } } switch (id.getIdType()) { @@ -172,6 +172,9 @@ void managedIdentityTest_SystemAssigned_SuccessfulResponse(ManagedIdentitySource IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(200, getSuccessfulResponse(resource))); @@ -206,6 +209,9 @@ void managedIdentityTest_UserAssigned_SuccessfulResponse(ManagedIdentitySourceTy IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource, id))).thenReturn(expectedResponse(200, getSuccessfulResponse(resource))); @@ -294,6 +300,9 @@ void managedIdentityTest_DifferentScopes_RequestsNewToken(ManagedIdentitySourceT IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(200, getSuccessfulResponse(resource))); when(httpClientMock.send(expectedRequest(source, anotherResource))).thenReturn(expectedResponse(200, getSuccessfulResponse(resource))); @@ -327,6 +336,9 @@ void managedIdentityTest_WrongScopes(ManagedIdentitySourceType source, String en IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } if (environmentVariables.getEnvironmentVariable("SourceType").equals(ManagedIdentitySourceType.CLOUD_SHELL.toString())) { when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(500, getMsiErrorResponseCloudShell())); @@ -364,7 +376,11 @@ void managedIdentityTest_WrongScopes(ManagedIdentitySourceType source, String en void managedIdentityTest_Retry(ManagedIdentitySourceType source, String endpoint, String resource) throws Exception { IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); - DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + + DefaultHttpClientManagedIdentity httpClientMock = mock(DefaultHttpClientManagedIdentity.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } miApp = ManagedIdentityApplication .builder(ManagedIdentityId.systemAssigned()) @@ -415,6 +431,9 @@ void managedIdentity_RequestFailed_NoPayload(ManagedIdentitySourceType source, S IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(500, "")); @@ -449,7 +468,9 @@ void managedIdentity_RequestFailed_NullResponse(ManagedIdentitySourceType source IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); - + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(200, "")); miApp = ManagedIdentityApplication @@ -483,6 +504,9 @@ void managedIdentity_RequestFailed_UnreachableNetwork(ManagedIdentitySourceType IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenThrow(new SocketException("A socket operation was attempted to an unreachable network.")); @@ -517,6 +541,9 @@ void managedIdentity_SharedCache(ManagedIdentitySourceType source, String endpoi IEnvironmentVariables environmentVariables = new EnvironmentVariablesHelper(source, endpoint); ManagedIdentityApplication.setEnvironmentVariables(environmentVariables); DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (source == SERVICE_FABRIC) { + ServiceFabricManagedIdentitySource.setHttpClient(httpClientMock); + } when(httpClientMock.send(expectedRequest(source, resource))).thenReturn(expectedResponse(200, getSuccessfulResponse(resource)));