You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2020/10/09 10:42:23 UTC
[james-project] 05/05: JAMES-3420 Avoid logging user password upon
webadmin user creation
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 0148ce3f5daadaf2d22c578174a733be9feca511
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Oct 8 12:38:05 2020 +0700
JAMES-3420 Avoid logging user password upon webadmin user creation
---
.../james/modules/server/DataRoutesModules.java | 4 ++
.../james/modules/server/WebAdminServerModule.java | 3 +
.../integration/WebAdminServerIntegrationTest.java | 30 ++++++++++
.../org/apache/james/webadmin/WebAdminServer.java | 11 ++--
.../james/webadmin/mdc/LoggingRequestFilter.java | 65 ++++++++++++++++------
.../apache/james/webadmin/mdc/RequestLogger.java | 28 ++++++++++
.../org/apache/james/webadmin/WebAdminUtils.java | 3 +-
.../routes/UserCreationRequestLogger.java} | 50 ++++++++---------
8 files changed, 147 insertions(+), 47 deletions(-)
diff --git a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
index d198069..1014264 100644
--- a/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
+++ b/server/container/guice/protocols/webadmin-data/src/main/java/org/apache/james/modules/server/DataRoutesModules.java
@@ -21,6 +21,7 @@ package org.apache.james.modules.server;
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.dto.MappingSourceModule;
+import org.apache.james.webadmin.mdc.RequestLogger;
import org.apache.james.webadmin.routes.AddressMappingRoutes;
import org.apache.james.webadmin.routes.AliasRoutes;
import org.apache.james.webadmin.routes.DomainMappingsRoutes;
@@ -29,6 +30,7 @@ import org.apache.james.webadmin.routes.ForwardRoutes;
import org.apache.james.webadmin.routes.GroupsRoutes;
import org.apache.james.webadmin.routes.MappingRoutes;
import org.apache.james.webadmin.routes.RegexMappingRoutes;
+import org.apache.james.webadmin.routes.UserCreationRequestLogger;
import org.apache.james.webadmin.routes.UserRoutes;
import org.apache.james.webadmin.utils.JsonTransformerModule;
@@ -52,5 +54,7 @@ public class DataRoutesModules extends AbstractModule {
Multibinder<JsonTransformerModule> jsonTransformerModuleMultibinder = Multibinder.newSetBinder(binder(), JsonTransformerModule.class);
jsonTransformerModuleMultibinder.addBinding().to(MappingSourceModule.class);
+
+ Multibinder.newSetBinder(binder(), RequestLogger.class).addBinding().to(UserCreationRequestLogger.class);
}
}
diff --git a/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java b/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java
index ff2f263..a2625be 100644
--- a/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java
+++ b/server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/WebAdminServerModule.java
@@ -46,6 +46,7 @@ import org.apache.james.webadmin.WebAdminServer;
import org.apache.james.webadmin.authentication.AuthenticationFilter;
import org.apache.james.webadmin.authentication.JwtFilter;
import org.apache.james.webadmin.authentication.NoAuthenticationFilter;
+import org.apache.james.webadmin.mdc.RequestLogger;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.apache.james.webadmin.utils.JsonTransformerModule;
import org.slf4j.Logger;
@@ -84,6 +85,8 @@ public class WebAdminServerModule extends AbstractModule {
Multibinder.newSetBinder(binder(), GuiceProbe.class).addBinding().to(WebAdminGuiceProbe.class);
Multibinder.newSetBinder(binder(), JsonTransformerModule.class);
+
+ Multibinder.newSetBinder(binder(), RequestLogger.class);
}
@Provides
diff --git a/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/WebAdminServerIntegrationTest.java b/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/WebAdminServerIntegrationTest.java
index 4d303a4..992a2e9 100644
--- a/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/WebAdminServerIntegrationTest.java
+++ b/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/WebAdminServerIntegrationTest.java
@@ -37,6 +37,7 @@ import org.apache.james.probe.DataProbe;
import org.apache.james.utils.DataProbeImpl;
import org.apache.james.utils.WebAdminGuiceProbe;
import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.mdc.LoggingRequestFilter;
import org.apache.james.webadmin.routes.AliasRoutes;
import org.apache.james.webadmin.routes.DomainsRoutes;
import org.apache.james.webadmin.routes.ForwardRoutes;
@@ -51,7 +52,11 @@ import org.apache.james.webadmin.swagger.routes.SwaggerRoutes;
import org.eclipse.jetty.http.HttpStatus;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
import io.restassured.RestAssured;
public abstract class WebAdminServerIntegrationTest {
@@ -175,6 +180,19 @@ public abstract class WebAdminServerIntegrationTest {
}
@Test
+ void putShouldNotLogThePassword() {
+ ListAppender<ILoggingEvent> loggingEvents = getListAppenderForClass(LoggingRequestFilter.class);
+
+ with()
+ .body("{\"password\":\"password\"}")
+ .put(SPECIFIC_USER);
+
+ assertThat(loggingEvents.list)
+ .hasSize(1)
+ .allSatisfy(loggingEvent -> assertThat(loggingEvent.getMDCPropertyMap()).doesNotContainKey("request-body"));
+ }
+
+ @Test
void deleteShouldRemoveTheUser() throws Exception {
dataProbe.addUser(USERNAME, "anyPassword");
@@ -366,4 +384,16 @@ public abstract class WebAdminServerIntegrationTest {
.body("status", is("completed"))
.body("type", is("MailboxesExportTask"));
}
+
+
+ public static ListAppender<ILoggingEvent> getListAppenderForClass(Class clazz) {
+ Logger logger = (Logger) LoggerFactory.getLogger(clazz);
+
+ ListAppender<ILoggingEvent> loggingEventListAppender = new ListAppender<>();
+ loggingEventListAppender.start();
+
+ logger.addAppender(loggingEventListAppender);
+
+ return loggingEventListAppender;
+ }
}
\ No newline at end of file
diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/WebAdminServer.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/WebAdminServer.java
index 1c9f00a..9a13bdcc 100644
--- a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/WebAdminServer.java
+++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/WebAdminServer.java
@@ -63,17 +63,20 @@ public class WebAdminServer implements Startable {
private final Service service;
private final AuthenticationFilter authenticationFilter;
private final MetricFactory metricFactory;
+ private final LoggingRequestFilter loggingRequestFilter;
@Inject
protected WebAdminServer(WebAdminConfiguration configuration,
- @Named("webAdminRoutes") List<Routes> routesList,
- AuthenticationFilter authenticationFilter,
- MetricFactory metricFactory) {
+ @Named("webAdminRoutes") List<Routes> routesList,
+ AuthenticationFilter authenticationFilter,
+ MetricFactory metricFactory,
+ LoggingRequestFilter loggingRequestFilter) {
this.configuration = configuration;
this.privateRoutes = privateRoutes(routesList);
this.publicRoutes = publicRoutes(routesList);
this.authenticationFilter = authenticationFilter;
this.metricFactory = metricFactory;
+ this.loggingRequestFilter = loggingRequestFilter;
this.service = Service.ignite();
}
@@ -116,7 +119,7 @@ public class WebAdminServer implements Startable {
private void configureMDC() {
service.before(new MDCFilter());
- service.before(new LoggingRequestFilter());
+ service.before(loggingRequestFilter);
service.after(new LoggingResponseFilter());
service.after(new MDCCleanupFilter());
}
diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java
index 32ef13e..06a7edf 100644
--- a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java
+++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java
@@ -21,6 +21,10 @@ package org.apache.james.webadmin.mdc;
import static org.apache.james.webadmin.authentication.AuthenticationFilter.LOGIN;
+import java.util.Set;
+
+import javax.inject.Inject;
+
import org.apache.james.util.MDCStructuredLogger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -32,13 +36,46 @@ import spark.Request;
import spark.Response;
public class LoggingRequestFilter implements Filter {
- private static final Logger LOGGER = LoggerFactory.getLogger(LoggingRequestFilter.class);
- static final String REQUEST_BODY = "request-body";
- static final String METHOD = "method";
- static final String ENDPOINT = "endpoint";
- static final String QUERY_PARAMETERS = "queryParameters";
- static final String IP = "ip";
- static final String REQUEST_ID = "requestId";
+ private static class DefaultRequestLogger implements RequestLogger {
+ private static final DefaultRequestLogger INSTANCE = new DefaultRequestLogger();
+
+ @Override
+ public boolean applies(Request request) {
+ return true;
+ }
+
+ @Override
+ public void log(Request request, RequestId requestId) {
+ MDCStructuredLogger.forLogger(LOGGER)
+ .addField(REQUEST_ID, requestId.asString())
+ .addField(IP, request.ip())
+ .addField(ENDPOINT, request.url())
+ .addField(METHOD, request.requestMethod())
+ .addField(LOGIN, request.attribute(LOGIN))
+ .addField(QUERY_PARAMETERS, ImmutableSet.copyOf(request.queryParams()))
+ .addField(REQUEST_BODY, request.body())
+ .log(logger -> logger.info("WebAdmin request received"));
+ }
+ }
+
+ public static final Logger LOGGER = LoggerFactory.getLogger(LoggingRequestFilter.class);
+ public static final String REQUEST_BODY = "request-body";
+ public static final String METHOD = "method";
+ public static final String ENDPOINT = "endpoint";
+ public static final String QUERY_PARAMETERS = "queryParameters";
+ public static final String IP = "ip";
+ public static final String REQUEST_ID = "requestId";
+
+ public static LoggingRequestFilter create() {
+ return new LoggingRequestFilter(ImmutableSet.of());
+ }
+
+ private final Set<RequestLogger> requestLoggers;
+
+ @Inject
+ public LoggingRequestFilter(Set<RequestLogger> requestLoggers) {
+ this.requestLoggers = requestLoggers;
+ }
@Override
public void handle(Request request, Response response) {
@@ -46,14 +83,10 @@ public class LoggingRequestFilter implements Filter {
request.attribute(REQUEST_ID, requestId);
- MDCStructuredLogger.forLogger(LOGGER)
- .addField(REQUEST_ID, requestId.asString())
- .addField(IP, request.ip())
- .addField(ENDPOINT, request.url())
- .addField(METHOD, request.requestMethod())
- .addField(LOGIN, request.attribute(LOGIN))
- .addField(QUERY_PARAMETERS, ImmutableSet.copyOf(request.queryParams()))
- .addField(REQUEST_BODY, request.body())
- .log(logger -> logger.info("WebAdmin request received"));
+ requestLoggers.stream()
+ .filter(requestLogger -> requestLogger.applies(request))
+ .findFirst()
+ .orElse(DefaultRequestLogger.INSTANCE)
+ .log(request, requestId);
}
}
diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/RequestLogger.java b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/RequestLogger.java
new file mode 100644
index 0000000..4d8ca10
--- /dev/null
+++ b/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/RequestLogger.java
@@ -0,0 +1,28 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.webadmin.mdc;
+
+import spark.Request;
+
+public interface RequestLogger {
+ boolean applies(Request request);
+
+ void log(Request request, RequestId requestId);
+}
diff --git a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/WebAdminUtils.java b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/WebAdminUtils.java
index 60aeb11..f63ac3f 100644
--- a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/WebAdminUtils.java
+++ b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/WebAdminUtils.java
@@ -32,6 +32,7 @@ import org.apache.james.metrics.tests.RecordingMetricFactory;
import org.apache.james.util.Port;
import org.apache.james.webadmin.authentication.AuthenticationFilter;
import org.apache.james.webadmin.authentication.NoAuthenticationFilter;
+import org.apache.james.webadmin.mdc.LoggingRequestFilter;
import com.google.common.collect.ImmutableList;
@@ -45,7 +46,7 @@ import reactor.util.retry.Retry;
public class WebAdminUtils {
private static class ConcurrentSafeWebAdminServer extends WebAdminServer {
ConcurrentSafeWebAdminServer(WebAdminConfiguration configuration, List<Routes> routesList, AuthenticationFilter authenticationFilter, MetricFactory metricFactory) {
- super(configuration, routesList, authenticationFilter, metricFactory);
+ super(configuration, routesList, authenticationFilter, metricFactory, LoggingRequestFilter.create());
}
/**
diff --git a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserCreationRequestLogger.java
similarity index 50%
copy from server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java
copy to server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserCreationRequestLogger.java
index 32ef13e..8bcbe5c 100644
--- a/server/protocols/webadmin/webadmin-core/src/main/java/org/apache/james/webadmin/mdc/LoggingRequestFilter.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserCreationRequestLogger.java
@@ -17,43 +17,41 @@
* under the License. *
****************************************************************/
-package org.apache.james.webadmin.mdc;
+package org.apache.james.webadmin.routes;
import static org.apache.james.webadmin.authentication.AuthenticationFilter.LOGIN;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.ENDPOINT;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.IP;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.LOGGER;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.METHOD;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.QUERY_PARAMETERS;
+import static org.apache.james.webadmin.mdc.LoggingRequestFilter.REQUEST_ID;
import org.apache.james.util.MDCStructuredLogger;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.james.webadmin.mdc.RequestId;
+import org.apache.james.webadmin.mdc.RequestLogger;
import com.google.common.collect.ImmutableSet;
-import spark.Filter;
import spark.Request;
-import spark.Response;
-
-public class LoggingRequestFilter implements Filter {
- private static final Logger LOGGER = LoggerFactory.getLogger(LoggingRequestFilter.class);
- static final String REQUEST_BODY = "request-body";
- static final String METHOD = "method";
- static final String ENDPOINT = "endpoint";
- static final String QUERY_PARAMETERS = "queryParameters";
- static final String IP = "ip";
- static final String REQUEST_ID = "requestId";
+// This class skips logging of the body for user creation requests as it contains the user password
+public class UserCreationRequestLogger implements RequestLogger {
@Override
- public void handle(Request request, Response response) {
- RequestId requestId = RequestId.random();
-
- request.attribute(REQUEST_ID, requestId);
+ public boolean applies(Request request) {
+ return request.pathInfo().startsWith(UserRoutes.USERS)
+ && request.requestMethod().equals("PUT");
+ }
+ @Override
+ public void log(Request request, RequestId requestId) {
MDCStructuredLogger.forLogger(LOGGER)
- .addField(REQUEST_ID, requestId.asString())
- .addField(IP, request.ip())
- .addField(ENDPOINT, request.url())
- .addField(METHOD, request.requestMethod())
- .addField(LOGIN, request.attribute(LOGIN))
- .addField(QUERY_PARAMETERS, ImmutableSet.copyOf(request.queryParams()))
- .addField(REQUEST_BODY, request.body())
- .log(logger -> logger.info("WebAdmin request received"));
+ .addField(REQUEST_ID, requestId.asString())
+ .addField(IP, request.ip())
+ .addField(ENDPOINT, request.url())
+ .addField(METHOD, request.requestMethod())
+ .addField(LOGIN, request.attribute(LOGIN))
+ .addField(QUERY_PARAMETERS, ImmutableSet.copyOf(request.queryParams()))
+ .log(logger -> logger.info("WebAdmin request received: user creation request"));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org