Fix user log entry of access log. (#5715)

helidon-io · Dec 19, 2022 · a45b04c · a45b04c
1 parent 1396836
commit a45b04c
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 15 deletions.
diff --git a/...server/access-log/src/main/java/io/helidon/nima/webserver/accesslog/AccessLogFeature.java b/...server/access-log/src/main/java/io/helidon/nima/webserver/accesslog/AccessLogFeature.java
@@ -92,7 +92,10 @@ public static Builder builder() {
 
     @Override
     public void setup(HttpRouting.Builder routing) {
-        routing.addFilter(this::filter);
+        // only add the filter if enabled, otherwise ignore
+        if (enabled) {
+            routing.addFilter(this::filter);
+        }
     }
 
     private void filter(FilterChain chain, RoutingRequest req, RoutingResponse res) {
@@ -102,9 +105,7 @@ private void filter(FilterChain chain, RoutingRequest req, RoutingResponse res)
         try {
             chain.proceed();
         } finally {
-            if (enabled) {
-                log(req, res, now, nanoNow);
-            }
+            log(req, res, now, nanoNow);
         }
     }
 

diff --git a/.../webserver/access-log/src/main/java/io/helidon/nima/webserver/accesslog/UserLogEntry.java b/.../webserver/access-log/src/main/java/io/helidon/nima/webserver/accesslog/UserLogEntry.java
@@ -16,17 +16,16 @@
 package io.helidon.nima.webserver.accesslog;
 
 import java.security.Principal;
+import java.util.Optional;
 
-import io.helidon.common.GenericType;
+import io.helidon.common.security.SecurityContext;
 
 /**
  * Access log entry for security username.
  * The username has a value only on successful authentication.
  * If there is no security configured, or the authentication fails, username is not available.
  */
 public final class UserLogEntry extends AbstractLogEntry {
-    private static final GenericType<Principal> PRINCIPAL_GENERIC_TYPE = GenericType.create(Principal.class);
-
     private UserLogEntry(Builder builder) {
         super(builder);
     }
@@ -50,14 +49,22 @@ public static Builder builder() {
         return new Builder();
     }
 
+    @SuppressWarnings({"unchecked", "rawtypes"})
     @Override
     protected String doApply(AccessLogContext context) {
-        // TODO this must be resolved - either through Context, or request attributes
-        //        return context.serverRequest()
-        //                .value(PRINCIPAL_GENERIC_TYPE)
-        //                .map(Principal::getName)
-        //                .orElse(NOT_AVAILABLE);
-        return NOT_AVAILABLE;
+        Optional<SecurityContext> maybeContext = context.serverRequest()
+                .context()
+                .get(SecurityContext.class);
+
+        if (maybeContext.isEmpty()) {
+            return NOT_AVAILABLE;
+        }
+
+        SecurityContext<Principal> securityContext = maybeContext.get();
+
+        return securityContext.userPrincipal()
+                .map(Principal::getName)
+                .orElse(NOT_AVAILABLE);
     }
 
     /**

diff --git a/nima/webserver/access-log/src/main/java/module-info.java b/nima/webserver/access-log/src/main/java/module-info.java
@@ -31,6 +31,7 @@
     requires java.logging;
 
     requires io.helidon.nima.webserver;
+    requires io.helidon.common.security;
 
     exports io.helidon.nima.webserver.accesslog;
 }
diff --git a/...er/access-log/src/test/java/io/helidon/nima/webserver/accesslog/AccessLogFeatureTest.java b/...er/access-log/src/test/java/io/helidon/nima/webserver/accesslog/AccessLogFeatureTest.java
@@ -16,14 +16,18 @@
 
 package io.helidon.nima.webserver.accesslog;
 
+import java.security.Principal;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
+import java.util.Optional;
 
+import io.helidon.common.context.Context;
 import io.helidon.common.http.Http;
 import io.helidon.common.http.Http.Header;
 import io.helidon.common.http.HttpPrologue;
 import io.helidon.common.http.ServerRequestHeaders;
 import io.helidon.common.http.WritableHeaders;
+import io.helidon.common.security.SecurityContext;
 import io.helidon.common.socket.PeerInfo;
 import io.helidon.common.uri.UriFragment;
 import io.helidon.common.uri.UriPath;
@@ -59,10 +63,19 @@ class AccessLogFeatureTest {
     void testHelidonFormat() {
         AccessLogFeature accessLog = AccessLogFeature.create();
 
+        Principal securityPrincipal = mock(Principal.class);
+        when(securityPrincipal.getName()).thenReturn("admin");
+        SecurityContext<Principal> securityContext = mock(SecurityContext.class);
+        when(securityContext.userPrincipal()).thenReturn(Optional.of(securityPrincipal));
+
+        Context requestContext = Context.create();
+        requestContext.register(securityContext);
+
         RoutingRequest request = mock(RoutingRequest.class);
         PeerInfo pi = mock(PeerInfo.class);
         when(pi.host()).thenReturn(REMOTE_IP);
         when(request.remotePeer()).thenReturn(pi);
+        when(request.context()).thenReturn(requestContext);
         HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
                                                     "HTTP",
                                                     "1.1",
@@ -88,7 +101,7 @@ void testHelidonFormat() {
 
         //192.168.0.104 - [18/Jun/2019:23:10:44 +0200] "GET /greet/test HTTP/1.1" 200 55 2248
 
-        String expected = REMOTE_IP + " - " + expectedTimestamp + " \"" + METHOD + " " + PATH + " " + HTTP_VERSION + "\" " +
+        String expected = REMOTE_IP + " admin " + expectedTimestamp + " \"" + METHOD + " " + PATH + " " + HTTP_VERSION + "\" " +
                 STATUS_CODE + " " + CONTENT_LENGTH + " " + TIME_TAKEN_MICROS;
 
         assertThat(logRecord, is(expected));
@@ -100,10 +113,19 @@ void testCommonFormat() {
                 .commonLogFormat()
                 .build();
 
+        Principal securityPrincipal = mock(Principal.class);
+        when(securityPrincipal.getName()).thenReturn("admin");
+        SecurityContext<Principal> securityContext = mock(SecurityContext.class);
+        when(securityContext.userPrincipal()).thenReturn(Optional.of(securityPrincipal));
+
+        Context requestContext = Context.create();
+        requestContext.register(securityContext);
+
         RoutingRequest request = mock(RoutingRequest.class);
         PeerInfo pi = mock(PeerInfo.class);
         when(pi.host()).thenReturn(REMOTE_IP);
         when(request.remotePeer()).thenReturn(pi);
+        when(request.context()).thenReturn(requestContext);
         HttpPrologue prologue = HttpPrologue.create("HTTP/1.1",
                                                     "HTTP",
                                                     "1.1",
@@ -129,7 +151,7 @@ void testCommonFormat() {
 
         //192.168.0.104 - [18/Jun/2019:23:10:44 +0200] "GET /greet/test HTTP/1.1" 200 55 2248
 
-        String expected = REMOTE_IP + " - - " + expectedTimestamp + " \"" + METHOD + " " + PATH + " " + HTTP_VERSION + "\" " +
+        String expected = REMOTE_IP + " - admin " + expectedTimestamp + " \"" + METHOD + " " + PATH + " " + HTTP_VERSION + "\" " +
                 STATUS_CODE + " " + CONTENT_LENGTH;
 
         assertThat(logRecord, is(expected));