Managed identity bug fix, pick up msal patch

billwert · billwert · commit b7f4d70e9376 · 2024-06-09T13:00:46.000-07:00
diff --git a/eng/versioning/external_dependencies.txt b/eng/versioning/external_dependencies.txt
@@ -201,7 +201,7 @@ com.microsoft.azure:azure-mgmt-resources;1.3.0
 com.microsoft.azure:azure-mgmt-search;1.24.1
 com.microsoft.azure:azure-mgmt-storage;1.3.0
 com.microsoft.azure:azure-storage;8.0.0
-com.microsoft.azure:msal4j;1.15.0
+com.microsoft.azure:msal4j;1.15.1
 com.microsoft.azure:msal4j-brokers;1.0.0
 com.microsoft.azure:msal4j-persistence-extension;1.3.0
 com.sun.activation:jakarta.activation;1.2.2
diff --git a/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml
@@ -64,7 +64,7 @@
     <dependency>
       <groupId>com.microsoft.azure</groupId>
       <artifactId>msal4j</artifactId>
-      <version>1.15.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
+      <version>1.15.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
       <scope>test</scope>
     </dependency>
     <dependency>
diff --git a/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml
@@ -68,7 +68,7 @@
       <dependency>
         <groupId>com.microsoft.azure</groupId>
         <artifactId>msal4j</artifactId>
-        <version>1.15.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
+        <version>1.15.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
         <scope>test</scope>
       </dependency>
       <dependency>
diff --git a/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml
@@ -77,7 +77,7 @@
     <dependency>
       <groupId>com.microsoft.azure</groupId>
       <artifactId>msal4j</artifactId>
-      <version>1.15.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
+      <version>1.15.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
       <scope>test</scope>
     </dependency>
     <dependency>
diff --git a/sdk/identity/azure-identity-broker/CHANGELOG.md b/sdk/identity/azure-identity-broker/CHANGELOG.md
@@ -7,6 +7,7 @@
 #### Dependency Updates
 
 - Upgraded `azure-identity` from `1.12.1` to version `1.12.2`.
+- Upgraded `msal4j` from `1.15.0` to version `1.15.1`.
 
 ## 1.1.1 (2024-05-02)
 
diff --git a/sdk/identity/azure-identity-broker/pom.xml b/sdk/identity/azure-identity-broker/pom.xml
@@ -37,7 +37,7 @@
     <dependency>
       <groupId>com.microsoft.azure</groupId>
       <artifactId>msal4j</artifactId>
-      <version>1.15.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
+      <version>1.15.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
     </dependency>
     <dependency>
       <groupId>com.microsoft.azure</groupId>
@@ -62,7 +62,7 @@
           <rules>
             <bannedDependencies>
               <includes>
-                <include>com.microsoft.azure:msal4j:[1.15.0]</include> <!-- {x-include-update;com.microsoft.azure:msal4j;external_dependency} -->
+                <include>com.microsoft.azure:msal4j:[1.15.1]</include> <!-- {x-include-update;com.microsoft.azure:msal4j;external_dependency} -->
                 <include>com.microsoft.azure:msal4j-brokers:[1.0.0]</include> <!-- {x-include-update;com.microsoft.azure:msal4j-brokers;external_dependency} -->
               </includes>
             </bannedDependencies>
diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md
@@ -11,6 +11,7 @@
 
 - Upgraded `azure-core` from `1.49.0` to version `1.49.1`.
 - Upgraded `azure-core-http-netty` from `1.15.0` to version `1.15.1`.
+- Upgraded `msal4j` from `1.15.0` to version `1.15.1`.
 
 ## 1.12.1 (2024-05-02)
 
diff --git a/sdk/identity/azure-identity/pom.xml b/sdk/identity/azure-identity/pom.xml
@@ -46,7 +46,7 @@
     <dependency>
       <groupId>com.microsoft.azure</groupId>
       <artifactId>msal4j</artifactId>
-      <version>1.15.0</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
+      <version>1.15.1</version> <!-- {x-version-update;com.microsoft.azure:msal4j;external_dependency} -->
     </dependency>
     <dependency>
       <groupId>com.microsoft.azure</groupId>
@@ -151,7 +151,7 @@
           <rules>
             <bannedDependencies>
               <includes>
-                <include>com.microsoft.azure:msal4j:[1.15.0]</include> <!-- {x-include-update;com.microsoft.azure:msal4j;external_dependency} -->
+                <include>com.microsoft.azure:msal4j:[1.15.1]</include> <!-- {x-include-update;com.microsoft.azure:msal4j;external_dependency} -->
                 <include>com.microsoft.azure:msal4j-persistence-extension:[1.3.0]</include> <!-- {x-include-update;com.microsoft.azure:msal4j-persistence-extension;external_dependency} -->
                 <include>net.java.dev.jna:jna-platform:[5.6.0]</include> <!-- {x-include-update;net.java.dev.jna:jna-platform;external_dependency} -->
                 <include>org.linguafranca.pwdb:KeePassJava2:[2.1.4]</include> <!-- {x-include-update;org.linguafranca.pwdb:KeePassJava2;external_dependency} -->
diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java
@@ -39,6 +39,7 @@
 import reactor.core.publisher.Mono;
 
 import javax.net.ssl.HttpsURLConnection;
+import java.io.File;
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
@@ -50,6 +51,7 @@
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.OffsetDateTime;
@@ -65,6 +67,8 @@
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import static com.azure.identity.implementation.util.ValidationUtil.validateSecretFile;
+
 /**
  * The identity client that contains APIs to retrieve access tokens
  * from various configurations.
@@ -956,8 +960,10 @@ private Mono<AccessToken> authenticateToArcManagedIdentityEndpoint(String identi
                         null));
                 }
 
-                String secretKeyPath = realm.substring(separatorIndex + 1);
-                secretKey = new String(Files.readAllBytes(Paths.get(secretKeyPath)), StandardCharsets.UTF_8);
+                String secretKeyPathHeaderValue = realm.substring(separatorIndex + 1);
+                Path secretKeyPath = validateSecretFile(new File(secretKeyPathHeaderValue), LOGGER);
+
+                secretKey = new String(Files.readAllBytes(secretKeyPath), StandardCharsets.UTF_8);
 
 
                 if (connection != null) {
diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java
@@ -97,6 +97,8 @@
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
+import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform;
+
 public abstract class IdentityClientBase {
     static final SerializerAdapter SERIALIZER_ADAPTER = JacksonAdapter.createDefaultSerializerAdapter();
     static final String WINDOWS_STARTER = "cmd.exe";
@@ -811,10 +813,6 @@ String getSafeWorkingDirectory() {
         }
     }
 
-    boolean isWindowsPlatform() {
-        return System.getProperty("os.name").contains("Windows");
-    }
-
     String redactInfo(String input) {
         return ACCESS_TOKEN_PATTERN.matcher(input).replaceAll("****");
     }
diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java
@@ -111,4 +111,13 @@ public static byte[] convertInputStreamToByteArray(InputStream inputStream) {
         }
         return outputStream.toByteArray();
     }
+
+
+    public static boolean isWindowsPlatform() {
+        return System.getProperty("os.name").contains("Windows");
+    }
+
+    public static boolean isLinuxPlatform() {
+        return System.getProperty("os.name").contains("Linux");
+    }
 }
diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java
@@ -3,10 +3,18 @@
 
 package com.azure.identity.implementation.util;
 
+import com.azure.core.exception.ClientAuthenticationException;
+import com.azure.core.util.CoreUtils;
 import com.azure.core.util.logging.ClientLogger;
 
+import java.io.File;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import static com.azure.identity.implementation.util.IdentityUtil.isLinuxPlatform;
+import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform;
 
 /**
  * Utility class for validating parameters.
@@ -91,4 +99,45 @@ public static void validateInteractiveBrowserRedirectUrlSetup(Integer port, Stri
     private static boolean isValidTenantCharacter(char c) {
         return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || (c == '.') || (c == '-');
     }
+
+
+    public static Path validateSecretFile(File file, ClientLogger logger) {
+
+        Path path = file.toPath();
+        if (isWindowsPlatform()) {
+            String programData = System.getenv("ProgramData");
+            if (CoreUtils.isNullOrEmpty(programData)) {
+                throw logger.logExceptionAsError(new ClientAuthenticationException("The ProgramData environment"
+                    + " variable is not set.", null));
+            }
+            String target = Paths.get(programData, "AzureConnectedMachineAgent", "Tokens").toString();
+            if (!path.getParent().toString().equals(target)) {
+                throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is not"
+                    + " located in the expected directory.", null));
+            }
+        } else if (isLinuxPlatform()) {
+            Path target = Paths.get("/", "var", "opt", "azcmagent", "tokens");
+            if (!path.getParent().equals(target)) {
+                throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is not"
+                    + " located in the expected directory.", null));
+            }
+        } else {
+            throw logger.logExceptionAsError(new ClientAuthenticationException("The platform is not supported"
+                + " for Azure Arc Managed Identity Endpoint", null));
+        }
+
+        if (!path.toString().endsWith(".key")) {
+            throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file does not"
+                + " have the expected file extension", null));
+        }
+
+
+
+        if (file.length() > 4096) {
+            throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is too large"
+                + " to be read from Azure Arc Managed Identity Endpoint", null));
+        }
+
+        return path;
+    }
 }
diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java
@@ -0,0 +1,97 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.identity.implementation;
+
+import com.azure.core.exception.ClientAuthenticationException;
+import com.azure.core.util.logging.ClientLogger;
+import com.azure.identity.implementation.util.ValidationUtil;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import static com.azure.identity.implementation.util.IdentityUtil.isLinuxPlatform;
+import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@DisabledOnOs({OS.MAC})
+public class ValidationUtilTests {
+    private static final ClientLogger LOGGER = new ClientLogger(ValidationUtilTests.class);
+
+    private static File good;
+    private static File fileTooLong;
+    private static File wrongPrefix;
+    private static File wrongExtension;
+    private static File fileWithRelativeSegments;
+
+
+    @BeforeAll
+    public static void setupClass() {
+        Path beginning = null;
+        if (isWindowsPlatform()) {
+            beginning = Paths.get(System.getenv("ProgramData"), "AzureConnectedMachineAgent", "Tokens");
+        } else if (isLinuxPlatform()) {
+
+            beginning = Paths.get("/", "var", "opt", "azcmagent", "tokens");
+        }
+
+        good = new TestFile(Paths.get(beginning.toString(), "good.key").toString());
+        fileTooLong = new TestFile(Paths.get(beginning.toString(), "fileTooLong.key").toString(), 4097);
+        wrongPrefix = new TestFile(Paths.get("wrongPrefix", ".key").toString());
+        wrongExtension = new TestFile(Paths.get(beginning.toString(), "wrongExtension.txt").toString());
+        fileWithRelativeSegments = new TestFile(Paths.get(beginning.toString(), "..", "file.key").toString());
+
+    }
+    @Test
+    public void testValidPath() {
+        assertDoesNotThrow(() -> ValidationUtil.validateSecretFile(good, LOGGER));
+    }
+
+    @Test
+    public void testInvalidTooLong() {
+        Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(fileTooLong, LOGGER));
+        assertTrue(thrown.getMessage().contains("The secret key file is too large"));
+    }
+
+    @Test
+    public void testInvalidWrongPrefix() {
+        Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(wrongPrefix, LOGGER));
+        assertTrue(thrown.getMessage().contains("The secret key file is not located in the expected directory"));
+    }
+
+    @Test
+    public void testInvalidWrongExtension() {
+        Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(wrongExtension, LOGGER));
+        assertTrue(thrown.getMessage().contains("The secret key file does not have the expected file extension"));
+    }
+
+    @Test
+    public void testInvalidRelativeSegments() {
+        Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(fileWithRelativeSegments, LOGGER));
+        assertTrue(thrown.getMessage().contains("The secret key file is not located in the expected directory"));
+    }
+
+    static class TestFile extends File {
+        long length = 4096;
+        TestFile(String pathname) {
+            super(pathname);
+        }
+
+        TestFile(String pathName, long length) {
+            super(pathName);
+            this.length = length;
+        }
+
+        @Override
+        public long length() {
+            return length;
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -97,6 +97,8 @@`
`97`	`97`	`import java.util.function.Supplier;`
`98`	`98`	`import java.util.regex.Pattern;`
`99`	`99`
	`100`	`+import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform;`
	`101`	`+`
`100`	`102`	`public abstract class IdentityClientBase {`
`101`	`103`	`static final SerializerAdapter SERIALIZER_ADAPTER = JacksonAdapter.createDefaultSerializerAdapter();`
`102`	`104`	`static final String WINDOWS_STARTER = "cmd.exe";`
`@@ -811,10 +813,6 @@ String getSafeWorkingDirectory() {`
`811`	`813`	`}`
`812`	`814`	`}`
`813`	`815`
`814`		`- boolean isWindowsPlatform() {`
`815`		`- return System.getProperty("os.name").contains("Windows");`
`816`		`- }`
`817`		`-`
`818`	`816`	`String redactInfo(String input) {`
`819`	`817`	`return ACCESS_TOKEN_PATTERN.matcher(input).replaceAll("****");`
`820`	`818`	`}`