add tests for web/AuthorizationFilter (#4639)

opengrok-web/src/main/java/org/opengrok/web/AuthorizationFilter.java

@@ -18,12 +18,13 @@
  */
 
 /*
- * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2020, Chris Fraire <[email protected]>.
  */
 package org.opengrok.web;
 
 import java.io.IOException;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -35,6 +36,7 @@
 import jakarta.servlet.ServletResponse;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import org.opengrok.indexer.configuration.IncludeFiles;
 import org.opengrok.indexer.configuration.Project;
 import org.opengrok.indexer.logger.LoggerFactory;
 import org.opengrok.indexer.web.Laundromat;
@@ -46,7 +48,7 @@ public class AuthorizationFilter implements Filter {
 
     @Override
     public void init(FilterConfig fc) {
-        // Empty since there is No specific init configuration.
+        // Empty since there is no specific init configuration.
     }
 
     /**
@@ -56,49 +58,50 @@ public void init(FilterConfig fc) {
      * so does not have to be exempted here.
      */
     @Override
-    public void doFilter(ServletRequest sr, ServletResponse sr1, FilterChain fc) throws IOException, ServletException {
-        HttpServletRequest httpReq = (HttpServletRequest) sr;
-        HttpServletResponse httpRes = (HttpServletResponse) sr1;
+    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain fc)
+            throws IOException, ServletException {
 
-        if (httpReq.getServletPath().startsWith(RestApp.API_PATH)) {
+        HttpServletRequest httpRequest = (HttpServletRequest) servletRequest;
+        HttpServletResponse httpResponse = (HttpServletResponse) servletResponse;
+
+        if (httpRequest.getServletPath().startsWith(RestApp.API_PATH)) {
             if (LOGGER.isLoggable(Level.FINER)) {
                 LOGGER.log(Level.FINER, "Allowing request to {0} in {1}",
-                        new Object[] {Laundromat.launderLog(httpReq.getServletPath()),
+                        new Object[] {Laundromat.launderLog(httpRequest.getServletPath()),
                                 AuthorizationFilter.class.getName()});
             }
-            fc.doFilter(sr, sr1);
+            fc.doFilter(servletRequest, servletResponse);
             return;
         }
 
-        PageConfig config = PageConfig.get(httpReq);
-
+        PageConfig config = PageConfig.get(httpRequest);
         Project p = config.getProject();
         if (p != null && !config.isAllowed(p)) {
             if (LOGGER.isLoggable(Level.INFO)) {
-                if (httpReq.getRemoteUser() != null) {
+                if (httpRequest.getRemoteUser() != null) {
                     LOGGER.log(Level.INFO, "Access denied for user ''{0}'' for URI: {1}",
-                            new Object[] {Laundromat.launderLog(httpReq.getRemoteUser()),
-                                    Laundromat.launderLog(httpReq.getRequestURI())});
+                            new Object[] {Laundromat.launderLog(httpRequest.getRemoteUser()),
+                                    Laundromat.launderLog(httpRequest.getRequestURI())});
                 } else {
                     LOGGER.log(Level.INFO, "Access denied for URI: {0}",
-                            Laundromat.launderLog(httpReq.getRequestURI()));
+                            Laundromat.launderLog(httpRequest.getRequestURI()));
                 }
             }
 
-            if (!config.getEnv().getIncludeFiles().getForbiddenIncludeFileContent(false).isEmpty()) {
-                sr.getRequestDispatcher("/eforbidden").forward(sr, sr1);
+            IncludeFiles includeFiles = config.getEnv().getIncludeFiles();
+            if (Objects.nonNull(includeFiles) && !includeFiles.getForbiddenIncludeFileContent(false).isEmpty()) {
+                servletRequest.getRequestDispatcher("/eforbidden").forward(servletRequest, servletResponse);
                 return;
             }
 
-            httpRes.sendError(HttpServletResponse.SC_FORBIDDEN, "Access forbidden");
+            httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, "Access forbidden");
             return;
         }
-        fc.doFilter(sr, sr1);
+        fc.doFilter(servletRequest, servletResponse);
     }
 
     @Override
     public void destroy() {
         // Empty since there is No specific destroy configuration.
     }
-
 }

opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

@@ -113,12 +113,12 @@
  * pages (how content gets generated) consistent and to document the request
  * parameters used.
  * <p>
- * General contract for this class (i.e. if not explicitly documented): no
- * method of this class changes neither the request nor the response.
+ * General contract for this class (i.e. if not explicitly documented):
+ * no method of this class changes neither the request nor the response.
  *
  * @author Jens Elkner
  */
-public final class PageConfig {
+public class PageConfig {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(PageConfig.class);
 
@@ -166,7 +166,7 @@ public final class PageConfig {
      */
     private final Scripts scripts = new Scripts();
 
-    private static final String ATTR_NAME = PageConfig.class.getCanonicalName();
+    static final String ATTR_NAME = PageConfig.class.getCanonicalName();
     private HttpServletRequest req;
 
     private final ExecutorService executor;

opengrok-web/src/test/java/org/opengrok/web/AuthorizationFilterTest.java

@@ -0,0 +1,137 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * See LICENSE.txt included in this distribution for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at LICENSE.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ */
+package org.opengrok.web;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.tuple.Pair;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.opengrok.indexer.configuration.Project;
+import org.opengrok.indexer.configuration.RuntimeEnvironment;
+import org.opengrok.indexer.web.DummyHttpServletRequest;
+import org.opengrok.indexer.web.Prefix;
+import org.opengrok.web.api.v1.RestApp;
+
+import java.io.IOException;
+import java.util.stream.Stream;
+
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Provides coverage for the {@link AuthorizationFilter} class.
+ */
+class AuthorizationFilterTest {
+    /**
+     * Test that requests that start with API path are let through.
+     */
+    @Test
+    void testApiPath() throws ServletException, IOException {
+        AuthorizationFilter filter = new AuthorizationFilter();
+        HttpServletRequest request = new DummyHttpServletRequest() {
+            @Override
+            public String getServletPath() {
+                return RestApp.API_PATH + "foo";
+            }
+        };
+        HttpServletResponse response = mock(HttpServletResponse.class);
+
+        FilterChain chain = mock(FilterChain.class);
+        // The init() method is currently empty, however it does not hurt to exercise it in case that changes.
+        FilterConfig filterConfig = mock(FilterConfig.class);
+        filter.init(filterConfig);
+        filter.doFilter(request, response, chain);
+
+        verify(chain).doFilter(request, response);
+        verify(response, never()).sendError(anyInt());
+        verify(response, never()).sendError(anyInt(), anyString());
+    }
+
+    private static Stream<Pair<Boolean, String>> getParamsForTestIsAllowed() {
+        return Stream.of(Pair.of(true, null),
+                Pair.of(true, "user"),
+                Pair.of(false, null),
+                Pair.of(false, "user"));
+    }
+
+    @ParameterizedTest
+    @MethodSource("getParamsForTestIsAllowed")
+    void testIsAllowed(Pair<Boolean, String> pair) throws ServletException, IOException {
+        AuthorizationFilter filter = new AuthorizationFilter();
+        PageConfig pageConfig = mock(PageConfig.class);
+        Project project = mock(Project.class);
+        when(pageConfig.getProject()).thenReturn(project);
+        when(pageConfig.isAllowed(project)).thenReturn(pair.getLeft());
+        when(pageConfig.getEnv()).thenReturn(RuntimeEnvironment.getInstance());
+        HttpServletRequest request = new DummyHttpServletRequest() {
+            @Override
+            public String getServletPath() {
+                return Prefix.DOWNLOAD_P.toString();
+            }
+
+            @Override
+            public String getRemoteUser() {
+                return pair.getRight();
+            }
+
+            @Override
+            public String getRequestURI() {
+                return "URI";
+            }
+
+            @Override
+            public Object getAttribute(String s) {
+                if (s.equals(PageConfig.ATTR_NAME)) {
+                    return pageConfig;
+                }
+
+                return "X";
+            }
+        };
+        HttpServletResponse response = mock(HttpServletResponse.class);
+
+        FilterChain chain = mock(FilterChain.class);
+        // The init() method is currently empty, however it does not hurt to exercise it in case that changes.
+        FilterConfig filterConfig = mock(FilterConfig.class);
+        filter.init(filterConfig);
+        filter.doFilter(request, response, chain);
+
+        if (pair.getLeft()) {
+            verify(chain).doFilter(request, response);
+            verify(response, never()).sendError(anyInt());
+            verify(response, never()).sendError(anyInt(), anyString());
+        } else {
+            verify(chain, never()).doFilter(request, response);
+            verify(response).sendError(HttpServletResponse.SC_FORBIDDEN, "Access forbidden");
+        }
+    }
+}