NO-JIRA improve log to not be sensitive to line return from user input

server/sonar-server-common/build.gradle

@@ -29,6 +29,7 @@ dependencies {
   testImplementation 'org.elasticsearch.plugin:transport-netty4-client'
   testImplementation 'ch.qos.logback:logback-core'
   testImplementation 'com.google.code.findbugs:jsr305'
+  testImplementation 'org.subethamail:subethasmtp'
   testImplementation 'com.squareup.okhttp3:mockwebserver'
   testImplementation 'com.squareup.okio:okio'
   testImplementation 'com.tngtech.java:junit-dataprovider'

server/sonar-server-common/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java

@@ -23,19 +23,20 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Pattern;
 import javax.annotation.CheckForNull;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.mail.Email;
 import org.apache.commons.mail.EmailException;
 import org.apache.commons.mail.HtmlEmail;
 import org.apache.commons.mail.SimpleEmail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sonar.api.config.EmailSettings;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationChannel;
 import org.sonar.api.user.User;
 import org.sonar.api.utils.SonarException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.user.UserDto;
@@ -64,6 +65,8 @@ public class EmailNotificationChannel extends NotificationChannel {
    */
   private static final int SOCKET_TIMEOUT = 30_000;
 
+  private static final Pattern PATTERN_LINE_BREAK = Pattern.compile("[\n\r]");
+
   /**
    * Email Header Field: "List-ID".
    * Value of this field should contain mailing list identifier as specified in <a href="http://tools.ietf.org/html/rfc2919">RFC 2919</a>.
@@ -211,7 +214,9 @@ public class EmailNotificationChannel extends NotificationChannel {
     Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
 
     try {
-      LOG.trace("Sending email: {}", emailMessage);
+      LOG.atTrace().setMessage("Sending email: {}")
+        .addArgument(() -> sanitizeLog(emailMessage.getMessage()))
+        .log();
       String host = resolveHost();
 
       Email email = createEmailWithMessage(emailMessage);
@@ -226,6 +231,10 @@ public class EmailNotificationChannel extends NotificationChannel {
     }
   }
 
+  private static String sanitizeLog(String message) {
+    return PATTERN_LINE_BREAK.matcher(message).replaceAll("_");
+  }
+
   private static Email createEmailWithMessage(EmailMessage emailMessage) throws EmailException {
     if (emailMessage.isHtml()) {
       return new HtmlEmail().setHtmlMsg(emailMessage.getMessage());

server/sonar-webserver-core/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java → server/sonar-server-common/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java

@@ -35,10 +35,14 @@ import javax.mail.internet.MimeMessage;
 import org.apache.commons.mail.EmailException;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.slf4j.event.Level;
 import org.sonar.api.config.EmailSettings;
 import org.sonar.api.notifications.Notification;
+import org.sonar.api.testfixtures.log.LogTester;
+import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.server.issue.notification.EmailMessage;
 import org.sonar.server.issue.notification.EmailTemplate;
 import org.sonar.server.notification.email.EmailNotificationChannel.EmailDeliveryRequest;
@@ -61,6 +65,8 @@ public class EmailNotificationChannelTest {
 
   private static final String SUBJECT_PREFIX = "[SONARQUBE]";
 
+  @Rule
+  public LogTester logTester = new LogTester();
 
   private Wiser smtpServer;
   private EmailSettings configuration;
@@ -68,6 +74,7 @@ public class EmailNotificationChannelTest {
 
   @Before
   public void setUp() {
+    logTester.setLevel(LoggerLevel.DEBUG);
     smtpServer = new Wiser(0);
     smtpServer.start();
 
@@ -117,6 +124,17 @@ public class EmailNotificationChannelTest {
     assertThat((String) email.getContent()).startsWith("This is a test message from SonarQube.\r\n\r\nMail sent from: http://nemo.sonarsource.org");
   }
 
+  @Test
+  public void sendTestEmailShouldSanitizeLog() throws Exception {
+    logTester.setLevel(LoggerLevel.TRACE);
+    configure();
+    underTest.sendTestEmail("user@nowhere", "Test Message from SonarQube", "This is a message \n containing line breaks \r that should be sanitized when logged.");
+
+    assertThat(logTester.logs(Level.TRACE)).isNotEmpty()
+      .contains("Sending email: This is a message _ containing line breaks _ that should be sanitized when logged.__Mail sent from: http://nemo.sonarsource.org");
+
+  }
+
   @Test
   public void shouldThrowAnExceptionWhenUnableToSendTestEmail() {
     configure();

server/sonar-webserver-auth/src/main/java/org/sonar/server/authentication/event/AuthenticationEventImpl.java

@@ -21,10 +21,11 @@ package org.sonar.server.authentication.event;
 
 import com.google.common.base.Joiner;
 import java.util.Collections;
+import java.util.regex.Pattern;
 import javax.annotation.Nullable;
-import org.sonar.api.server.http.HttpRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.sonar.api.server.http.HttpRequest;
 import org.sonar.core.util.stream.MoreCollectors;
 
 import static java.util.Objects.requireNonNull;
@@ -32,18 +33,20 @@ import static java.util.Objects.requireNonNull;
 public class AuthenticationEventImpl implements AuthenticationEvent {
   private static final Logger LOGGER = LoggerFactory.getLogger("auth.event");
   private static final int FLOOD_THRESHOLD = 128;
+  private static final Pattern PATTERN_LINE_BREAK = Pattern.compile("[\n\r]");
 
   @Override
   public void loginSuccess(HttpRequest request, @Nullable String login, Source source) {
     checkRequest(request);
     requireNonNull(source, "source can't be null");
-    if (!LOGGER.isDebugEnabled()) {
-      return;
-    }
-    LOGGER.debug("login success [method|{}][provider|{}|{}][IP|{}|{}][login|{}]",
-      source.getMethod(), source.getProvider(), source.getProviderName(),
-      request.getRemoteAddr(), getAllIps(request),
-      preventLogFlood(emptyIfNull(login)));
+    LOGGER.atDebug().setMessage("login success [method|{}][provider|{}|{}][IP|{}|{}][login|{}]")
+      .addArgument(source::getMethod)
+      .addArgument(source::getProvider)
+      .addArgument(source::getProviderName)
+      .addArgument(request::getRemoteAddr)
+      .addArgument(() -> getAllIps(request))
+      .addArgument(() -> preventLogFlood(sanitizeLog(emptyIfNull(login))))
+      .log();
   }
 
   private static String getAllIps(HttpRequest request) {
@@ -103,4 +106,8 @@ public class AuthenticationEventImpl implements AuthenticationEvent {
     return str;
   }
 
+  private static String sanitizeLog(String message) {
+    return PATTERN_LINE_BREAK.matcher(message).replaceAll("_");
+  }
+
 }

server/sonar-webserver-auth/src/test/java/org/sonar/server/authentication/event/AuthenticationEventImplTest.java

@@ -82,7 +82,17 @@ public class AuthenticationEventImplTest {
 
     underTest.loginSuccess(request, "login", Source.sso());
 
-    verifyNoInteractions(request);
+    assertThat(logTester.logs()).isEmpty();
+  }
+
+  @Test
+  public void login_success_message_is_sanitized() {
+    logTester.setLevel(LoggerLevel.DEBUG);
+
+    underTest.loginSuccess(mockRequest("1.2.3.4"), "login with \n malicious line \r return", Source.sso());
+
+    assertThat(logTester.logs()).isNotEmpty()
+      .contains("login success [method|SSO][provider|SSO|sso][IP|1.2.3.4|][login|login with _ malicious line _ return]");
   }
 
   @Test