From 9234a58514677a11b4ed680af855e22cd5d538ce Mon Sep 17 00:00:00 2001 From: "antoine.vinot" Date: Thu, 25 Jan 2024 10:46:18 +0100 Subject: [PATCH] SONAR-21227 Disable log of user login --- .../sonar/process/logging/LogbackHelper.java | 2 +- .../process/logging/LogbackJsonLayout.java | 9 ++++- .../process/logging/RootLoggerConfig.java | 14 +++++++ .../process/logging/LogbackHelperTest.java | 4 +- .../logging/LogbackJsonLayoutTest.java | 17 +++++++++ .../server/app/WebServerProcessLogging.java | 38 +++++++++++++++---- .../app/WebServerProcessLoggingTest.java | 21 +++++++++- 7 files changed, 91 insertions(+), 14 deletions(-) diff --git a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java index 72c696dcd80..4b7d3d3dfcb 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java +++ b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java @@ -243,7 +243,7 @@ public class LogbackHelper extends AbstractLogHelper { public Encoder createJsonEncoder(LoggerContext context, RootLoggerConfig config) { LayoutWrappingEncoder encoder = new LayoutWrappingEncoder<>(); - encoder.setLayout(new LogbackJsonLayout(config.getProcessId().getKey(), config.getNodeNameField())); + encoder.setLayout(new LogbackJsonLayout(config.getProcessId().getKey(), config.getNodeNameField(), config.getExcludedFields())); encoder.setContext(context); encoder.start(); return encoder; diff --git a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackJsonLayout.java b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackJsonLayout.java index 6caa17969cc..83c6b91214b 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackJsonLayout.java +++ b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackJsonLayout.java @@ -30,6 +30,7 @@ import java.io.StringWriter; import java.time.Instant; import java.time.ZoneId; import java.time.format.DateTimeFormatter; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.regex.Pattern; @@ -52,10 +53,16 @@ public class LogbackJsonLayout extends LayoutBase { private final String processKey; private final String nodeName; + private final List exclusions; public LogbackJsonLayout(String processKey, String nodeName) { + this(processKey, nodeName, List.of()); + } + + public LogbackJsonLayout(String processKey, String nodeName, List exclusions) { this.processKey = requireNonNull(processKey); this.nodeName = nodeName; + this.exclusions = exclusions; } String getProcessKey() { @@ -72,7 +79,7 @@ public class LogbackJsonLayout extends LayoutBase { } json.name("process").value(processKey); for (Map.Entry entry : event.getMDCPropertyMap().entrySet()) { - if (entry.getValue() != null) { + if (entry.getValue() != null && !exclusions.contains(entry.getKey())) { json.name(entry.getKey()).value(entry.getValue()); } } diff --git a/server/sonar-process/src/main/java/org/sonar/process/logging/RootLoggerConfig.java b/server/sonar-process/src/main/java/org/sonar/process/logging/RootLoggerConfig.java index 41c0d6ee6ab..c065e42af8c 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/logging/RootLoggerConfig.java +++ b/server/sonar-process/src/main/java/org/sonar/process/logging/RootLoggerConfig.java @@ -19,6 +19,8 @@ */ package org.sonar.process.logging; +import java.util.Collection; +import java.util.List; import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -30,11 +32,13 @@ public final class RootLoggerConfig { private final ProcessId processId; private final String threadIdFieldPattern; private final String nodeNameField; + private final List excludedFields; private RootLoggerConfig(Builder builder) { this.processId = requireNonNull(builder.processId); this.threadIdFieldPattern = builder.threadIdFieldPattern; this.nodeNameField = Optional.ofNullable(builder.nodeNameField).orElse(""); + this.excludedFields = Optional.ofNullable(builder.excludedFields).orElse(List.of()); } public static Builder newRootLoggerConfigBuilder() { @@ -53,11 +57,16 @@ public final class RootLoggerConfig { return threadIdFieldPattern; } + public List getExcludedFields() { + return excludedFields; + } + public static final class Builder { @CheckForNull private ProcessId processId; private String threadIdFieldPattern = ""; private String nodeNameField; + private List excludedFields; private Builder() { // prevents instantiation outside RootLoggerConfig, use static factory method @@ -78,6 +87,11 @@ public final class RootLoggerConfig { return this; } + public Builder setExcludedFields(Collection excludedFields) { + this.excludedFields = excludedFields.stream().toList(); + return this; + } + public RootLoggerConfig build() { return new RootLoggerConfig(this); } diff --git a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java index 45382ba390f..754f3ee5b64 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java @@ -582,8 +582,8 @@ public class LogbackHelperTest { @Test public void createJsonEncoder_shouldStartJsonEncoder() { - RootLoggerConfig config = newRootLoggerConfigBuilder().setProcessId(WEB_SERVER).build(); - LogbackJsonLayout expectedJsonLayout = new LogbackJsonLayout(config.getProcessId().getKey(), config.getNodeNameField()); + RootLoggerConfig config = newRootLoggerConfigBuilder().setProcessId(WEB_SERVER).setExcludedFields(List.of("LOGIN")).build(); + LogbackJsonLayout expectedJsonLayout = new LogbackJsonLayout(config.getProcessId().getKey(), config.getNodeNameField(), List.of("LOGIN")); Encoder result = underTest.createJsonEncoder(underTest.getRootContext(), config); diff --git a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackJsonLayoutTest.java b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackJsonLayoutTest.java index 4d9d146584c..8d4e04c864d 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackJsonLayoutTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackJsonLayoutTest.java @@ -24,6 +24,7 @@ import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.LoggingEvent; import com.google.gson.Gson; import java.time.Instant; +import java.util.List; import org.junit.Test; import org.slf4j.LoggerFactory; import org.slf4j.MDC; @@ -151,6 +152,22 @@ public class LogbackJsonLayoutTest { } } + @Test + public void doLayout_whenMDC_shouldNotContainExcludedFields() { + try { + LogbackJsonLayout logbackJsonLayout = new LogbackJsonLayout("web", "", List.of("fromMdc")); + LoggingEvent event = new LoggingEvent("org.foundation.Caller", (Logger) LoggerFactory.getLogger("the.logger"), Level.WARN, "the message", null, new Object[0]); + MDC.put("fromMdc", "foo"); + + String log = logbackJsonLayout.doLayout(event); + + JsonLog json = new Gson().fromJson(log, JsonLog.class); + assertThat(json.fromMdc).isNull(); + } finally { + MDC.clear(); + } + } + private static class JsonLog { private String process; private String timestamp; diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/app/WebServerProcessLogging.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/app/WebServerProcessLogging.java index 6a8536b9b95..e4c6d48c3f1 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/app/WebServerProcessLogging.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/app/WebServerProcessLogging.java @@ -26,18 +26,22 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.FileAppender; import ch.qos.logback.core.encoder.Encoder; -import org.sonar.process.ProcessId; +import java.util.Collection; +import java.util.List; import org.sonar.process.Props; import org.sonar.process.logging.LogDomain; import org.sonar.process.logging.LogLevelConfig; import org.sonar.process.logging.RootLoggerConfig; import org.sonar.server.log.ServerProcessLogging; +import static org.apache.commons.lang.StringUtils.EMPTY; import static org.apache.commons.lang.StringUtils.isBlank; +import static org.sonar.process.ProcessId.WEB_SERVER; import static org.sonar.process.ProcessProperties.Property.LOG_JSON_OUTPUT; import static org.sonar.process.logging.AbstractLogHelper.PREFIX_LOG_FORMAT; import static org.sonar.process.logging.AbstractLogHelper.SUFFIX_LOG_FORMAT; import static org.sonar.process.logging.LogbackHelper.DEPRECATION_LOGGER_NAME; +import static org.sonar.process.logging.RootLoggerConfig.newRootLoggerConfigBuilder; import static org.sonar.server.authentication.UserSessionInitializer.USER_LOGIN_MDC_KEY; import static org.sonar.server.platform.web.logging.EntrypointMDCStorage.ENTRYPOINT_MDC_KEY; import static org.sonar.server.platform.web.requestid.RequestIdMDCStorage.HTTP_REQUEST_ID_MDC_KEY; @@ -49,16 +53,18 @@ public class WebServerProcessLogging extends ServerProcessLogging { private static final String DEPRECATION_LOG_FILE_PREFIX = "deprecation"; + private static final String ENABLE_LOGIN_PROPERTY = "sonar.deprecationLogs.loginEnabled"; + public WebServerProcessLogging() { - super(ProcessId.WEB_SERVER, "%X{" + HTTP_REQUEST_ID_MDC_KEY + "}"); + super(WEB_SERVER, "%X{" + HTTP_REQUEST_ID_MDC_KEY + "}"); } @Override protected void extendLogLevelConfiguration(LogLevelConfig.Builder logLevelConfigBuilder) { - logLevelConfigBuilder.levelByDomain("sql", ProcessId.WEB_SERVER, LogDomain.SQL); - logLevelConfigBuilder.levelByDomain("es", ProcessId.WEB_SERVER, LogDomain.ES); - logLevelConfigBuilder.levelByDomain("auth.event", ProcessId.WEB_SERVER, LogDomain.AUTH_EVENT); - JMX_RMI_LOGGER_NAMES.forEach(loggerName -> logLevelConfigBuilder.levelByDomain(loggerName, ProcessId.WEB_SERVER, LogDomain.JMX)); + logLevelConfigBuilder.levelByDomain("sql", WEB_SERVER, LogDomain.SQL); + logLevelConfigBuilder.levelByDomain("es", WEB_SERVER, LogDomain.ES); + logLevelConfigBuilder.levelByDomain("auth.event", WEB_SERVER, LogDomain.AUTH_EVENT); + JMX_RMI_LOGGER_NAMES.forEach(loggerName -> logLevelConfigBuilder.levelByDomain(loggerName, WEB_SERVER, LogDomain.JMX)); logLevelConfigBuilder.offUnlessTrace("org.apache.catalina.core.ContainerBase"); logLevelConfigBuilder.offUnlessTrace("org.apache.catalina.core.StandardContext"); @@ -72,10 +78,25 @@ public class WebServerProcessLogging extends ServerProcessLogging { configureDeprecatedApiLogger(props); } + @Override + protected RootLoggerConfig buildRootLoggerConfig(Props props) { + return getRootLoggerConfigBuilder(props, List.of(USER_LOGIN_MDC_KEY)).build(); + } + + private static RootLoggerConfig.Builder getRootLoggerConfigBuilder(Props props, Collection excludedFields) { + return newRootLoggerConfigBuilder() + .setProcessId(WEB_SERVER) + .setNodeNameField(getNodeNameWhenCluster(props)) + .setThreadIdFieldPattern("%X{" + HTTP_REQUEST_ID_MDC_KEY + "}") + .setExcludedFields(excludedFields.stream().toList()); + } + private void configureDeprecatedApiLogger(Props props) { LoggerContext context = helper.getRootContext(); - RootLoggerConfig config = buildRootLoggerConfig(props); + boolean isLoginEnabled = props.valueAsBoolean(ENABLE_LOGIN_PROPERTY, false); + RootLoggerConfig config = getRootLoggerConfigBuilder(props, isLoginEnabled ? List.of() : List.of(USER_LOGIN_MDC_KEY)).build(); + Encoder encoder = props.valueAsBoolean(LOG_JSON_OUTPUT.getKey(), Boolean.parseBoolean(LOG_JSON_OUTPUT.getDefaultValue())) ? helper.createJsonEncoder(context, config) : helper.createPatternLayoutEncoder(context, buildDepractedLogPatrern(config)); @@ -90,11 +111,12 @@ public class WebServerProcessLogging extends ServerProcessLogging { } private static String buildDepractedLogPatrern(RootLoggerConfig config) { + String userLoginPattern = " %X{" + USER_LOGIN_MDC_KEY + "}"; return PREFIX_LOG_FORMAT + (isBlank(config.getNodeNameField()) ? "" : (config.getNodeNameField() + " ")) + config.getProcessId().getKey() + "[" + config.getThreadIdFieldPattern() + "]" - + " %X{" + USER_LOGIN_MDC_KEY + "}" + + (config.getExcludedFields().contains(USER_LOGIN_MDC_KEY) ? EMPTY : userLoginPattern) + " %X{" + ENTRYPOINT_MDC_KEY + "}" + SUFFIX_LOG_FORMAT; } diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java index d09fbad5d47..1b79e31203f 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java @@ -32,10 +32,14 @@ import ch.qos.logback.core.encoder.Encoder; import ch.qos.logback.core.encoder.LayoutWrappingEncoder; import ch.qos.logback.core.joran.spi.JoranException; import com.google.common.collect.ImmutableList; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.stream.Stream; import org.junit.AfterClass; @@ -43,6 +47,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; import org.sonar.process.Props; import org.sonar.process.logging.LogbackHelper; import org.sonar.process.logging.LogbackJsonLayout; @@ -53,6 +58,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.slf4j.Logger.ROOT_LOGGER_NAME; import static org.sonar.process.ProcessProperties.Property.PATH_LOGS; +@RunWith(DataProviderRunner.class) public class WebServerProcessLoggingTest { @Rule @@ -525,9 +531,20 @@ public class WebServerProcessLoggingTest { assertThat(((LayoutWrappingEncoder) encoder).getLayout()).isInstanceOf(LogbackJsonLayout.class); } + @DataProvider + public static Object[][] configuration() { + return new Object[][] { + {Map.of("sonar.deprecationLogs.loginEnabled", "true"), "%d{yyyy.MM.dd HH:mm:ss} %-5level web[%X{HTTP_REQUEST_ID}] %X{LOGIN} %X{ENTRYPOINT} %msg%n"}, + {Map.of("sonar.deprecationLogs.loginEnabled", "false"), "%d{yyyy.MM.dd HH:mm:ss} %-5level web[%X{HTTP_REQUEST_ID}] %X{ENTRYPOINT} %msg%n"}, + {Map.of(), "%d{yyyy.MM.dd HH:mm:ss} %-5level web[%X{HTTP_REQUEST_ID}] %X{ENTRYPOINT} %msg%n"}, + }; + } + @Test - public void configure_whenJsonPropFalse_shouldConfigureDeprecatedLoggerWithPatternLayout() { + @UseDataProvider("configuration") + public void configure_whenJsonPropFalse_shouldConfigureDeprecatedLoggerWithPatternLayout(Map additionalProps, String expectedPattern) { props.set("sonar.log.jsonOutput", "false"); + additionalProps.forEach(props::set); LoggerContext context = underTest.configure(props); @@ -540,7 +557,7 @@ public class WebServerProcessLoggingTest { Encoder encoder = fileAppender.getEncoder(); assertThat(encoder).isInstanceOf(PatternLayoutEncoder.class); PatternLayoutEncoder patternLayoutEncoder = (PatternLayoutEncoder) encoder; - assertThat(patternLayoutEncoder.getPattern()).isEqualTo("%d{yyyy.MM.dd HH:mm:ss} %-5level web[%X{HTTP_REQUEST_ID}] %X{LOGIN} %X{ENTRYPOINT} %msg%n"); + assertThat(patternLayoutEncoder.getPattern()).isEqualTo(expectedPattern); } @Test -- 2.39.5