NIFI-12813 Corrected Username handling in HTTP Request Log

- Corrected Jetty AuthenticationState interface reference for authenticated user attribute mapping - Added unit test verifying expected attribute values Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com> This closes #8423.
apache · Feb 19, 2024 · 9a81f66 · 9a81f66
1 parent 2007d79
commit 9a81f66
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 2 deletions.
diff --git a/.../nifi-jetty/src/main/java/org/apache/nifi/web/server/log/RequestAuthenticationFilter.java b/.../nifi-jetty/src/main/java/org/apache/nifi/web/server/log/RequestAuthenticationFilter.java
@@ -17,7 +17,7 @@
 package org.apache.nifi.web.server.log;
 
 import org.apache.nifi.web.security.log.AuthenticationUserAttribute;
-import org.eclipse.jetty.security.AuthenticationState;
+import org.eclipse.jetty.server.Request.AuthenticationState;
 import org.eclipse.jetty.security.authentication.LoginAuthenticator;
 import org.eclipse.jetty.security.internal.DefaultUserIdentity;
 import org.eclipse.jetty.security.UserIdentity;
@@ -65,7 +65,7 @@ protected void doFilterInternal(final HttpServletRequest httpServletRequest, fin
             final String username = usernameAttribute.toString();
             final Principal principal = new UserPrincipal(username);
             final UserIdentity userIdentity = new DefaultUserIdentity(DEFAULT_SUBJECT, principal, DEFAULT_ROLES);
-            final AuthenticationState.Succeeded authenticationState = new LoginAuthenticator.UserAuthenticationSucceeded(METHOD, userIdentity);
+            final AuthenticationState authenticationState = new LoginAuthenticator.UserAuthenticationSucceeded(METHOD, userIdentity);
             httpServletRequest.setAttribute(AuthenticationState.class.getName(), authenticationState);
         }
     }

diff --git a/...i-jetty/src/test/java/org/apache/nifi/web/server/log/RequestAuthenticationFilterTest.java b/...i-jetty/src/test/java/org/apache/nifi/web/server/log/RequestAuthenticationFilterTest.java
@@ -0,0 +1,93 @@
+/*
+ * 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.nifi.web.server.log;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.nifi.web.security.log.AuthenticationUserAttribute;
+import org.eclipse.jetty.server.Request;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.io.IOException;
+import java.security.Principal;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class RequestAuthenticationFilterTest {
+
+    private static final String USERNAME = UserPrincipal.class.getSimpleName();
+
+    private static final String AUTHENTICATION_STATE = Request.AuthenticationState.class.getName();
+
+    @Mock
+    HttpServletRequest request;
+
+    @Mock
+    HttpServletResponse response;
+
+    @Mock
+    FilterChain filterChain;
+
+    @Captor
+    ArgumentCaptor<Request.AuthenticationState> authenticationStateCaptor;
+
+    private RequestAuthenticationFilter filter;
+
+    @BeforeEach
+    void setFilter() {
+        filter = new RequestAuthenticationFilter();
+    }
+
+    @Test
+    void testDoFilterInternalUsernameNotFound() throws ServletException, IOException {
+        when(request.getAttribute(eq(AuthenticationUserAttribute.USERNAME.getName()))).thenReturn(null);
+
+        filter.doFilterInternal(request, response, filterChain);
+
+        verify(request).getRemoteAddr();
+        verifyNoMoreInteractions(request);
+    }
+
+    @Test
+    void testDoFilterInternalUsernameFound() throws ServletException, IOException {
+        when(request.getAttribute(eq(AuthenticationUserAttribute.USERNAME.getName()))).thenReturn(USERNAME);
+
+        filter.doFilterInternal(request, response, filterChain);
+
+        verify(request).setAttribute(eq(AUTHENTICATION_STATE), authenticationStateCaptor.capture());
+
+        final Request.AuthenticationState authenticationState = authenticationStateCaptor.getValue();
+        assertNotNull(authenticationState);
+        final Principal principal = authenticationState.getUserPrincipal();
+        assertNotNull(principal);
+        assertEquals(USERNAME, principal.getName());
+    }
+}