Merge pull request #4193 from vespa-engine/bjorncs/user-principal-access-logging

Bjorncs/user principal access logging

7 files changed, 57 insertions, 10 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
index 2268b568b18..21e492fe57e 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
@@ -16,6 +16,7 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
+import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -71,6 +72,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
 
     private final HeaderFields trailers = new HeaderFields();
     private final Map<String, List<String>> parameters = new HashMap<>();
+    private Principal principal;
     private final long connectedAt;
     private Method method;
     private Version version;
@@ -294,6 +296,14 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
         return version == Version.HTTP_1_1;
     }
 
+    public Principal getUserPrincipal() {
+        return principal;
+    }
+
+    public void setUserPrincipal(Principal principal) {
+        this.principal = principal;
+    }
+
     public static HttpRequest newServerRequest(CurrentContainer container, URI uri) {
         return newServerRequest(container, uri, Method.GET);
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java
index a46d35f8e70..617f0cbd184 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java
@@ -36,14 +36,12 @@ public abstract class DiscFilterRequest {
     protected static final String HTTPS_PREFIX = "https";
     protected static final int DEFAULT_HTTP_PORT = 80;
     protected static final int DEFAULT_HTTPS_PORT = 443;
-    private static final String JDISC_REQUEST_PRINCIPAL = "jdisc.request.principal";
 
     private final ServletOrJdiscHttpRequest parent;
     protected final InetSocketAddress localAddress;
     protected final Map<String, List<String>> untreatedParams;
     private final HeaderFields untreatedHeaders;
     private List<Cookie> untreatedCookies = null;
-    private Principal userPrincipal = null;
     private String remoteUser = null;
     private String[] roles = null;
     private boolean overrideIsUserInRole = false;
@@ -330,9 +328,7 @@ public abstract class DiscFilterRequest {
         return port;
     }
 
-    public Principal getUserPrincipal() {
-        return (Principal) getAttribute(JDISC_REQUEST_PRINCIPAL);
-    }
+    public abstract Principal getUserPrincipal();
 
     public boolean isSecure() {
         if(getScheme().equalsIgnoreCase(HTTPS_PREFIX)) {
@@ -375,9 +371,7 @@ public abstract class DiscFilterRequest {
         this.remoteUser = remoteUser;
     }
 
-    public void setUserPrincipal(Principal principal) {
-        setAttribute(JDISC_REQUEST_PRINCIPAL, principal);
-    }
+    public abstract void setUserPrincipal(Principal principal);
 
     public void setUserRoles(String[] roles) {
         this.roles = roles;
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java
index 1e9d09ecb17..07e3b97ba90 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java
@@ -5,6 +5,7 @@ import com.yahoo.jdisc.http.HttpHeaders;
 import com.yahoo.jdisc.http.HttpRequest;
 
 import java.net.URI;
+import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -103,6 +104,16 @@ public class JdiscFilterRequest extends DiscFilterRequest {
     }
 
     @Override
+    public Principal getUserPrincipal() {
+        return parent.getUserPrincipal();
+    }
+
+    @Override
+    public void setUserPrincipal(Principal principal) {
+        this.parent.setUserPrincipal(principal);
+    }
+
+    @Override
     public void clearCookies() {
         parent.headers().remove(HttpHeaders.Names.COOKIE);
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java
index 0fd52d3f12a..11c2baf0176 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java
@@ -6,6 +6,7 @@ import com.yahoo.jdisc.http.servlet.ServletRequest;
 
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
+import java.security.Principal;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashSet;
@@ -128,6 +129,16 @@ class ServletFilterRequest extends DiscFilterRequest {
     }
 
     @Override
+    public Principal getUserPrincipal() {
+        return parent.getUserPrincipal();
+    }
+
+    @Override
+    public void setUserPrincipal(Principal principal) {
+        parent.setUserPrincipal(principal);
+    }
+
+    @Override
     public void removeHeaders(String name) {
         parent.removeHeaders(name);
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
index fcdfb877bfa..c3c83474e56 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java
@@ -5,6 +5,7 @@ import com.google.common.base.Objects;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.AccessLogEntry;
 
+import com.yahoo.jdisc.http.servlet.ServletRequest;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.RequestLog;
 import org.eclipse.jetty.server.Response;
@@ -17,6 +18,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
+import java.security.Principal;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -109,6 +111,10 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog
         accessLogEntry.setHttpVersion(request.getProtocol());
         accessLogEntry.setScheme(request.getScheme());
         accessLogEntry.setLocalPort(request.getLocalPort());
+        Principal principal = (Principal) request.getAttribute(ServletRequest.JDISC_REQUEST_PRINCIPAL);
+        if (principal != null) {
+            accessLogEntry.setUserPrincipal(principal);
+        }
     }
 
     private static String getRemoteAddress(final HttpServletRequest request) {
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLoggingRequestHandler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLoggingRequestHandler.java
index 43513b4efba..e30d50ecdbf 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLoggingRequestHandler.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLoggingRequestHandler.java
@@ -54,24 +54,27 @@ public class AccessLoggingRequestHandler extends AbstractRequestHandler {
         Preconditions.checkArgument(request instanceof HttpRequest, "Expected HttpRequest, got " + request);
         final HttpRequest httpRequest = (HttpRequest) request;
         httpRequest.context().put(CONTEXT_KEY_ACCESS_LOG_ENTRY, accessLogEntry);
-        final ResponseHandler accessLoggingResponseHandler = new AccessLoggingResponseHandler(handler, accessLogEntry);
+        final ResponseHandler accessLoggingResponseHandler = new AccessLoggingResponseHandler(httpRequest, handler, accessLogEntry);
         final ContentChannel requestContentChannel = delegate.handleRequest(request, accessLoggingResponseHandler);
         return requestContentChannel;
     }
 
     private static class AccessLoggingResponseHandler implements ResponseHandler {
+        private final HttpRequest request;
         private final ResponseHandler delegateHandler;
         private final AccessLogEntry accessLogEntry;
 
         public AccessLoggingResponseHandler(
-                final ResponseHandler delegateHandler,
+                HttpRequest request, final ResponseHandler delegateHandler,
                 final AccessLogEntry accessLogEntry) {
+            this.request = request;
             this.delegateHandler = delegateHandler;
             this.accessLogEntry = accessLogEntry;
         }
 
         @Override
         public ContentChannel handleResponse(Response response) {
+            accessLogEntry.setUserPrincipal(request.getUserPrincipal());
             return delegateHandler.handleResponse(response);
         }
 
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java
index 3cbe415d39d..db8780b087c 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java
@@ -12,6 +12,7 @@ import javax.servlet.http.HttpServletRequestWrapper;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
+import java.security.Principal;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -36,6 +37,7 @@ import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnection;
  * @since 5.27
  */
 public class ServletRequest extends HttpServletRequestWrapper implements ServletOrJdiscHttpRequest {
+    public static final String JDISC_REQUEST_PRINCIPAL = "jdisc.request.principal";
 
     private final HttpServletRequest request;
     private final HeaderFields headerFields;
@@ -252,4 +254,14 @@ public class ServletRequest extends HttpServletRequestWrapper implements Servlet
     public long getConnectedAt(TimeUnit unit) {
         return unit.convert(connectedAt, TimeUnit.MILLISECONDS);
     }
+
+    @Override
+    public Principal getUserPrincipal() {
+        // NOTE: The principal from the underlying servlet request is ignored. JDisc filters are the source-of-truth.
+        return (Principal) request.getAttribute(JDISC_REQUEST_PRINCIPAL);
+    }
+
+    public void setUserPrincipal(Principal principal) {
+        request.setAttribute(JDISC_REQUEST_PRINCIPAL, principal);
+    }
 }