Skip to content

Commit 8e66e62

Browse files
Avery-DunnAvery-Dunn
committed
Address PR feedback
1 parent 1a12e5b commit 8e66e62

3 files changed

Lines changed: 153 additions & 6 deletions

File tree

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
*/
1414
class ManagedIdentityClient {
1515
private static final Logger LOG = LoggerFactory.getLogger(ManagedIdentityClient.class);
16-
private static final String WINDOWS_HIMDS_FILEPATH = "%Programfiles%\\AzureConnectedMachineAgent\\himds.exe";
17-
private static final String LINUX_HIMDS_FILEPATH = "/opt/azcmagent/bin/himds";
16+
private static String WINDOWS_HIMDS_FILEPATH = "%Programfiles%\\AzureConnectedMachineAgent\\himds.exe";
17+
private static String LINUX_HIMDS_FILEPATH = "/opt/azcmagent/bin/himds";
1818

1919
static ManagedIdentitySourceType getManagedIdentitySource() {
2020
IEnvironmentVariables environmentVariables = AbstractManagedIdentitySource.getEnvironmentVariables();
@@ -47,6 +47,10 @@ static ManagedIdentitySourceType getManagedIdentitySource() {
4747
}
4848
}
4949

50+
ManagedIdentityClient(){
51+
// Default constructor for testing
52+
}
53+
5054
ManagedIdentityResponse getManagedIdentityResponse(ManagedIdentityParameters parameters) {
5155
return managedIdentitySource.getManagedIdentityResponse(parameters);
5256
}
@@ -72,7 +76,7 @@ private static AbstractManagedIdentitySource createManagedIdentitySource(MsalReq
7276
static boolean validateAzureArcEnvironment(IEnvironmentVariables environmentVariables) {
7377
if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.IDENTITY_ENDPOINT)) &&
7478
!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.IMDS_ENDPOINT))) {
75-
LOG.debug("[Managed Identity] Azure Arc managed identity is available through environment variables.");
79+
LOG.info("[Managed Identity] Azure Arc managed identity is available through environment variables.");
7680
return true;
7781
}
7882

@@ -81,20 +85,31 @@ static boolean validateAzureArcEnvironment(IEnvironmentVariables environmentVari
8185
if (osName.contains("windows")) {
8286
File windowsFile = new File(WINDOWS_HIMDS_FILEPATH);
8387
if (windowsFile.exists()) {
84-
LOG.debug("[Managed Identity] Azure Arc managed identity is available through file detection.");
88+
LOG.info("[Managed Identity] Azure Arc managed identity is available through file detection.");
8589
return true;
8690
}
8791
} else if (osName.contains("linux")) {
8892
File linuxFile = new File(LINUX_HIMDS_FILEPATH);
8993
if (linuxFile.exists()) {
90-
LOG.debug("[Managed Identity] Azure Arc managed identity is available through file detection.");
94+
LOG.info("[Managed Identity] Azure Arc managed identity is available through file detection.");
9195
return true;
9296
}
9397
} else {
9498
LOG.warn("[Managed Identity] Azure Arc managed identity cannot be configured on a platform other than Windows and Linux.");
9599
}
96100

97-
LOG.debug("[Managed Identity] Azure Arc managed identity is not available.");
101+
LOG.info("[Managed Identity] Azure Arc managed identity is not available.");
98102
return false;
99103
}
104+
105+
//These set methods should be used solely for automated testing.
106+
// -The file paths are not normally customizable in this flow, as they should exist in an Azure Arc environment.
107+
// -However, unit tests need some way to adjust them as part of mocking the environment and creating temporary files.
108+
void setWindowsFilePath(String filePath) {
109+
WINDOWS_HIMDS_FILEPATH = filePath;
110+
}
111+
112+
void setLinuxFilePath(String filePath) {
113+
LINUX_HIMDS_FILEPATH = filePath;
114+
}
100115
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,8 @@ public class EnvironmentVariablesHelper implements IEnvironmentVariables {
4444
public String getEnvironmentVariable(String envVariable) {
4545
return mockedEnvironmentVariables.get(envVariable);
4646
}
47+
48+
void setEnvironmentVariable(String key, String value) {
49+
mockedEnvironmentVariables.put(key, value);
50+
}
4751
}

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
package com.microsoft.aad.msal4j;
55

66
import com.nimbusds.oauth2.sdk.util.URLUtils;
7+
import org.junit.jupiter.api.BeforeEach;
78
import org.junit.jupiter.api.Nested;
89
import org.junit.jupiter.api.Test;
910
import org.junit.jupiter.api.TestInstance;
1011
import org.junit.jupiter.api.extension.ExtendWith;
12+
import org.junit.jupiter.api.io.TempDir;
1113
import org.junit.jupiter.params.ParameterizedTest;
1214
import org.junit.jupiter.params.provider.MethodSource;
1315
import org.junit.jupiter.params.provider.ValueSource;
1416
import org.mockito.junit.jupiter.MockitoExtension;
1517

18+
import java.io.File;
19+
import java.io.IOException;
1620
import java.net.SocketException;
1721
import java.nio.file.Path;
1822
import java.nio.file.Paths;
@@ -32,6 +36,7 @@
3236
import static java.util.Collections.*;
3337
import static org.apache.http.HttpStatus.*;
3438
import static org.junit.jupiter.api.Assertions.*;
39+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
3540
import static org.mockito.ArgumentMatchers.any;
3641
import static org.mockito.Mockito.*;
3742

@@ -961,4 +966,127 @@ private void assertMsalServiceException(String errorCode, String message) throws
961966
assertTrue(ex.getMessage().contains(message));
962967
}
963968
}
969+
970+
971+
@Nested
972+
class OSTests {
973+
974+
@TempDir
975+
Path tempDir;
976+
977+
private ManagedIdentityClient client = new ManagedIdentityClient();
978+
private EnvironmentVariablesHelper envVars;
979+
980+
@BeforeEach
981+
void setUp() {
982+
envVars = new EnvironmentVariablesHelper(AZURE_ARC, azureArcEndpoint);
983+
ManagedIdentityApplication.setEnvironmentVariables(envVars);
984+
}
985+
986+
@Test
987+
void validateAzureArc_WithCorrectEnvironmentVariables() {
988+
// Set environment variables for Azure Arc
989+
envVars.setEnvironmentVariable(Constants.IDENTITY_ENDPOINT, "https://example.com");
990+
envVars.setEnvironmentVariable(Constants.IMDS_ENDPOINT, "https://example2.com");
991+
992+
// Test validation
993+
boolean result = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
994+
995+
assertTrue(result, "Azure Arc should be validated with correct environment variables");
996+
}
997+
998+
@Test
999+
void validateAzureArc_WithMissingEnvironmentVariables() {
1000+
// Only set one environment variable
1001+
envVars.setEnvironmentVariable(Constants.IDENTITY_ENDPOINT, "https://example.com");
1002+
envVars.setEnvironmentVariable(Constants.IMDS_ENDPOINT, null);
1003+
1004+
boolean result = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1005+
1006+
assertFalse(result, "Azure Arc validation should fail with missing environment variables");
1007+
}
1008+
1009+
@Test
1010+
void validateAzureArc_WindowsFileExists() throws IOException {
1011+
// Determine OS and skip if not Windows
1012+
String osName = System.getProperty("os.name").toLowerCase();
1013+
assumeTrue(osName.contains("windows"), "Test only runs on Windows");
1014+
1015+
// Create temp file to simulate Azure Arc file on Windows
1016+
File windowsFile = tempDir.resolve("himds.key").toFile();
1017+
assertTrue(windowsFile.createNewFile(), "Failed to create test file");
1018+
1019+
// Set custom file path for testing
1020+
client.setWindowsFilePath(windowsFile.getAbsolutePath());
1021+
1022+
boolean result = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1023+
1024+
assertTrue(result, "Azure Arc should be validated when Windows file exists");
1025+
}
1026+
1027+
@Test
1028+
void validateAzureArc_LinuxFileExists() throws IOException {
1029+
// Determine OS and skip if not Linux
1030+
String osName = System.getProperty("os.name").toLowerCase();
1031+
assumeTrue(osName.contains("linux"), "Test only runs on Linux");
1032+
1033+
// Create temp file to simulate Azure Arc file on Linux
1034+
File linuxFile = tempDir.resolve("himds.sock").toFile();
1035+
assertTrue(linuxFile.createNewFile(), "Failed to create test file");
1036+
1037+
// Set custom file path for testing
1038+
client.setLinuxFilePath(linuxFile.getAbsolutePath());
1039+
1040+
boolean result = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1041+
1042+
assertTrue(result, "Azure Arc should be validated when Linux file exists");
1043+
}
1044+
1045+
@Test
1046+
void validateAzureArc_FilesNotExist() {
1047+
envVars.setEnvironmentVariable(Constants.IMDS_ENDPOINT, null);
1048+
1049+
// Set non-existent file paths
1050+
client.setWindowsFilePath(tempDir.resolve("nonexistent-himds.key").toString());
1051+
client.setLinuxFilePath(tempDir.resolve("nonexistent-himds.sock").toString());
1052+
1053+
boolean result = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1054+
1055+
assertFalse(result, "Azure Arc validation should fail when files don't exist");
1056+
}
1057+
1058+
@Test
1059+
void validateAzureArc_CrossPlatformTest() throws IOException {
1060+
// This test creates both Windows and Linux files to test the method
1061+
// independent of platform in unit tests
1062+
envVars.setEnvironmentVariable(Constants.IMDS_ENDPOINT, null);
1063+
// Create both temp files
1064+
File windowsFile = tempDir.resolve("himds.key").toFile();
1065+
File linuxFile = tempDir.resolve("himds.sock").toFile();
1066+
1067+
// Set custom file paths for testing
1068+
client.setWindowsFilePath(windowsFile.getAbsolutePath());
1069+
client.setLinuxFilePath(linuxFile.getAbsolutePath());
1070+
1071+
// Test with no files
1072+
boolean noFilesResult = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1073+
assertFalse(noFilesResult, "Validation should fail when no files exist");
1074+
1075+
// Create Windows file
1076+
assertTrue(windowsFile.createNewFile(), "Failed to create Windows test file");
1077+
1078+
// The result depends on OS - but at least one path should be checked
1079+
boolean windowsFileResult = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1080+
1081+
// Create Linux file
1082+
assertTrue(linuxFile.createNewFile(), "Failed to create Linux test file");
1083+
1084+
// Now with both files existing, the result should depend on OS
1085+
boolean bothFilesResult = ManagedIdentityClient.validateAzureArcEnvironment(envVars);
1086+
1087+
// At least one of the tests with files should pass
1088+
assertTrue(windowsFileResult || bothFilesResult,
1089+
"At least one validation should succeed when test files exist");
1090+
}
1091+
}
9641092
}

0 commit comments

Comments
 (0)