Merge branch 'main' into PR #1492 to update

Author: lqiu96

src/main/java/com/google/cloud/logging/logback/LoggingAppender.java

@@ -23,6 +23,7 @@
 import ch.qos.logback.core.UnsynchronizedAppenderBase;
 import ch.qos.logback.core.util.Loader;
 import com.google.api.core.InternalApi;
+import com.google.api.core.ObsoleteApi;
 import com.google.auth.oauth2.GoogleCredentials;
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.logging.Instrumentation;
@@ -35,10 +36,12 @@
 import com.google.cloud.logging.Payload;
 import com.google.cloud.logging.Severity;
 import com.google.cloud.logging.Synchronicity;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
-import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -138,6 +141,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase<ILoggingEvent> {
   private String log;
   private String resourceType;
   private String credentialsFile;
+  private GoogleCredentials credentials;
   private String logDestinationProjectId;
   private boolean autoPopulateMetadata = true;
   private boolean redirectToStdout = false;
@@ -185,17 +189,46 @@ public void setResourceType(String resourceType) {
   }
 
   /**
-   * Sets the path to the <a
+   * This method is obsolete because of a potential security risk. Use the {@link
+   * #setCredentials(GoogleCredentials)} method instead.
+   *
+   * <p>If you know that you will be loading credential configurations of a specific type, it is
+   * recommended to use a credential-type-specific `fromStream()` method. This will ensure that an
+   * unexpected credential type with potential for malicious intent is not loaded unintentionally.
+   * You might still have to do validation for certain credential types. Please follow the
+   * recommendation for that method.
+   *
+   * <p>If you are loading your credential configuration from an untrusted source and have not
+   * mitigated the risks (e.g. by validating the configuration yourself), make these changes as soon
+   * as possible to prevent security risks to your environment.
+   *
+   * <p>Regardless of the method used, it is always your responsibility to validate configurations
+   * received from external sources.
+   *
+   * <p>Sets the path to the <a
    * href="https://cloud.google.com/iam/docs/creating-managing-service-account-keys">credential
    * file</a>. If not set the appender will use {@link GoogleCredentials#getApplicationDefault()} to
    * authenticate.
    *
    * @param credentialsFile the path to the credentials file.
    */
+  @ObsoleteApi(
+      "This method is obsolete because of a potential security risk. Use the setCredentials() method instead")
   public void setCredentialsFile(String credentialsFile) {
     this.credentialsFile = credentialsFile;
   }
 
+  /**
+   * Sets the credential to use. If not set the appender will use {@link
+   * GoogleCredentials#getApplicationDefault()} to authenticate.
+   *
+   * @param credentials the GoogleCredentials to set
+   */
+  public void setCredentials(GoogleCredentials credentials) {
+    Preconditions.checkNotNull(credentials, "Credentials cannot be null");
+    this.credentials = credentials;
+  }
+
   /**
    * Sets project ID to be used to customize log destination name for written log entries.
    *
@@ -445,10 +478,12 @@ protected LoggingOptions getLoggingOptions() {
     if (loggingOptions == null) {
       LoggingOptions.Builder builder = LoggingOptions.newBuilder();
       builder.setProjectId(logDestinationProjectId);
-      if (!Strings.isNullOrEmpty(credentialsFile)) {
+      if (credentials != null) {
+        builder.setCredentials(credentials);
+      } else if (!Strings.isNullOrEmpty(credentialsFile)) {
         try {
           builder.setCredentials(
-              GoogleCredentials.fromStream(new FileInputStream(credentialsFile)));
+              GoogleCredentials.fromStream(Files.newInputStream(Paths.get(credentialsFile))));
         } catch (IOException e) {
           throw new RuntimeException(
               String.format(

src/main/resources/META-INF/native-image/com.google.cloud/google-cloud-logging-logback/reflect-config.json

@@ -35,6 +35,7 @@
       {"name":"<init>","parameterTypes":[] },
       {"name":"setAutoPopulateMetadata","parameterTypes":["boolean"] },
       {"name":"setCredentialsFile","parameterTypes":["java.lang.String"] },
+      {"name":"setCredentials","parameterTypes":["com.google.auth.oauth2.GoogleCredentials"] },
       {"name":"setFlushLevel","parameterTypes":["ch.qos.logback.classic.Level"] },
       {"name":"setLog","parameterTypes":["java.lang.String"] },
       {"name":"setLogDestinationProjectId","parameterTypes":["java.lang.String"] },

src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java

@@ -23,11 +23,13 @@
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 
 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.filter.ThresholdFilter;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.classic.spi.LoggingEvent;
+import com.google.auth.oauth2.GoogleCredentials;
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.Timestamp;
 import com.google.cloud.logging.Instrumentation;
@@ -60,7 +62,7 @@
 public class LoggingAppenderTest {
   private static final String PROJECT_ID = "test-project";
   private static final String CRED_FILE_PROJECT_ID = "project-12345";
-  private static final String OVERRIDED_PROJECT_ID = "some-project-id";
+  private static final String OVERRIDDEN_PROJECT_ID = "some-project-id";
   private static final String DUMMY_CRED_FILE_PATH =
       "src/test/java/com/google/cloud/logging/logback/dummy-credentials.json";
   private static final Payload.JsonPayload JSON_PAYLOAD =
@@ -289,6 +291,22 @@ public void testMdcValuesAreConvertedToLabels() {
     assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(INFO_ENTRY);
   }
 
+  @Test
+  public void testCreateLoggingOptionsWithValidCredentials() {
+    LoggingAppender appender = new LoggingAppender();
+    appender.setCredentials(GoogleCredentials.newBuilder().build());
+    // ServiceOptions requires a projectId to be set. Normally this is determined by the
+    // GoogleCredentials (Credential set above is a dummy value with no ProjectId).
+    appender.setLogDestinationProjectId(PROJECT_ID);
+    appender.getLoggingOptions();
+  }
+
+  @Test
+  public void testCreateLoggingOptionsWithNullCredentials() {
+    LoggingAppender appender = new LoggingAppender();
+    assertThrows(NullPointerException.class, () -> appender.setCredentials(null));
+  }
+
   @Test(expected = RuntimeException.class)
   public void testCreateLoggingOptionsWithInvalidCredentials() {
     final String nonExistentFile = "/path/to/non/existent/file";
@@ -310,8 +328,8 @@ public void testCreateLoggingOptionsWithDestination() {
     // Try to build LoggingOptions with file based credentials.
     LoggingAppender appender = new LoggingAppender();
     appender.setCredentialsFile(DUMMY_CRED_FILE_PATH);
-    appender.setLogDestinationProjectId(OVERRIDED_PROJECT_ID);
-    assertThat(appender.getLoggingOptions().getProjectId()).isEqualTo(OVERRIDED_PROJECT_ID);
+    appender.setLogDestinationProjectId(OVERRIDDEN_PROJECT_ID);
+    assertThat(appender.getLoggingOptions().getProjectId()).isEqualTo(OVERRIDDEN_PROJECT_ID);
   }
 
   private LoggingEvent createLoggingEvent(Level level, long timestamp) {