Redirect to dex only for browsers, otherwise return 401.

Adds a test to validate that all endpoints require auth

Author: shartte

src/main/java/net/neoforged/meta/security/BrowserAwareAuthenticationEntryPoint.java

@@ -0,0 +1,49 @@
+package net.neoforged.meta.security;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.springframework.http.MediaType;
+import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.web.AuthenticationEntryPoint;
+import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
+
+import java.io.IOException;
+
+/**
+ * Custom authentication entry point that redirects browsers to OAuth login
+ * but returns 401 Unauthorized for non-browser clients (API clients).
+ * <p>
+ * Determines if a request is from a browser by checking if the Accept header
+ * contains "text/html".
+ */
+public class BrowserAwareAuthenticationEntryPoint implements AuthenticationEntryPoint {
+
+    private final LoginUrlAuthenticationEntryPoint browserEntryPoint;
+
+    public BrowserAwareAuthenticationEntryPoint(String loginFormUrl) {
+        this.browserEntryPoint = new LoginUrlAuthenticationEntryPoint(loginFormUrl);
+    }
+
+    @Override
+    public void commence(HttpServletRequest request, HttpServletResponse response,
+                         AuthenticationException authException) throws IOException, ServletException {
+
+        // Check if the request accepts HTML (indicates a browser)
+        if (isBrowserDocumentRequest(request)) {
+            // Redirect to OAuth login page for browsers
+            browserEntryPoint.commence(request, response, authException);
+        } else {
+            // Return 401 Unauthorized for API clients
+            response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");
+        }
+    }
+
+    /**
+     * We heuristically consider any request that explicitly asks for text/html without weighting a browser request.
+     */
+    private boolean isBrowserDocumentRequest(HttpServletRequest request) {
+        var mediaTypes = MediaType.parseMediaTypes(request.getHeader("Accept"));
+        return mediaTypes.contains(MediaType.TEXT_HTML);
+    }
+}

src/main/java/net/neoforged/meta/security/SecurityConfiguration.java

@@ -78,6 +78,9 @@ public SecurityFilterChain uiSecurityFilterChain(HttpSecurity http) {
                 .sessionManagement(session -> session
                         .sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)
                 )
+                .exceptionHandling(exceptions -> exceptions
+                        .authenticationEntryPoint(new BrowserAwareAuthenticationEntryPoint("/oauth2/authorization/dex"))
+                )
                 .build();
     }
 }

src/test/java/net/neoforged/meta/TestAuthenticatedEndpoints.java

@@ -0,0 +1,97 @@
+package net.neoforged.meta;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.web.server.LocalServerPort;
+import org.springframework.http.HttpMethod;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.client.RestClient;
+import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping;
+import org.springframework.web.util.UrlPathHelper;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
+@ActiveProfiles("test")
+public class TestAuthenticatedEndpoints {
+
+    @LocalServerPort
+    private int port;
+
+    @ParameterizedTest
+    @MethodSource("provideEndpoints")
+    void testAllEndpointsAuthenticatedForApiConsumers(String method, String path) throws Exception {
+        var restClient = RestClient.create();
+        var baseUrl = "http://localhost:" + port;
+
+        try (var response = restClient
+                .method(HttpMethod.valueOf(method))
+                .uri(baseUrl + path)
+                .exchange((_, clientResponse) -> clientResponse)) {
+            int statusCode = response.getStatusCode().value();
+            assertEquals(401, statusCode,
+                    "Expected 401 Unauthorized for " + method + " " + path + " but got " + statusCode);
+        }
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideEndpoints")
+    void testAllEndpointsAuthenticatedForBrowsers(String method, String path) throws Exception {
+        var restClient = RestClient.create();
+        var baseUrl = "http://localhost:" + port;
+
+        try (var response = restClient
+                .method(HttpMethod.valueOf(method))
+                .uri(baseUrl + path)
+                .header("Accept", "text/html")
+                .exchange((_, clientResponse) -> clientResponse)) {
+            int statusCode = response.getStatusCode().value();
+            assertEquals(302, statusCode,
+                    "Expected redirect for " + method + " " + path + " but got " + statusCode);
+            String location = response.getHeaders().getFirst("Location");
+            assertNotNull(location, "location");
+            location = new UrlPathHelper().removeSemicolonContent(location); // sometimes ;jsessionid= is appended
+            assertEquals(baseUrl + "/oauth2/authorization/dex", location);
+        }
+    }
+
+    /**
+     * Automatically discover all registered API endpoints from Spring.
+     */
+    static Stream<Arguments> provideEndpoints(
+            @Autowired RequestMappingHandlerMapping requestMappingHandlerMapping) {
+        return requestMappingHandlerMapping.getHandlerMethods().entrySet().stream()
+                .flatMap(entry -> {
+                    var mapping = entry.getKey();
+                    var patterns = new HashSet<>(mapping.getPatternValues());
+                    var httpMethods = mapping.getMethodsCondition().getMethods();
+
+                    patterns.remove("/"); // The index page is white-listed to be accessible anonymously
+
+                    // If no methods specified, default to GET
+                    final var finalMethods = httpMethods.isEmpty()
+                            ? Set.of(RequestMethod.GET)
+                            : httpMethods;
+
+                    return patterns.stream()
+                            .flatMap(pattern -> finalMethods.stream()
+                                    .map(requestMethod -> {
+                                        // Replace path variables with dummy values
+                                        var resolvedPath = pattern.replaceAll("\\{[^}]+}", "123");
+                                        return Arguments.of(requestMethod.name(), resolvedPath);
+                                    }));
+                })
+                .distinct();
+    }
+}